Page MenuHomePhabricator

Visibility change to CategoryPage::mCategoryViewerClass breaks extension
Closed, ResolvedPublic


The [0] change arbitrarily modified the visibility of the mCategoryViewerClass property which in turn breaks extensions [1, 2] that rely on the existing visibility.

As for comparison, REL1_29 passes [3] all unit and integration tests while the MW master branch [2] returns with:

PHP Fatal error:  Access level to SIL\Category\LanguageFilterCategoryPage::$mCategoryViewerClass must be public (as in class CategoryPage) in /home/travis/build/SemanticMediaWiki/mw/extensions/SemanticInterlanguageLinks/src/Category/LanguageFilterCategoryPage.php on line 18

The argument in T166483 is understood but changing the visibility is not a solution as one cannot catch the issue while trying to maintain compatibility [4] with REL1_27.

I'd rather see that a new public method is introduced (e.g.CategoryPage::getCategoryViewerClass) which would allow subsequent implementations to override the return value without the need to modify the CategoryPage::mCategoryViewerClass property.


Event Timeline

@Legoktm It would be marvellous if could take a peek at this.

Legoktm added subscribers: matmarex, Louperivois.

Since the purpose of T166483: allow CategoryPageView to set $mCategoryViewerClass was to make it modifyable via a hook, we probably want a getter and a setter.

Change 368594 had a related patch set uploaded (by Louperivois; owner: Louperivois):
[mediawiki/core@master] Implement a getter/setter for mCategoryViewerClass

Change 368594 merged by jenkins-bot:
[mediawiki/core@master] Implement a getter/setter for mCategoryViewerClass

matmarex assigned this task to Louperivois.
matmarex removed a project: Patch-For-Review.

Sorry about the issue and thanks for the bug report, @mwjames. And thank you for stepping up to fix it, @Louperivois.