Page MenuHomePhabricator

Visibility of WikiPage::$mPreparedEdit changed from public to protected in 1.22.1 tarball, breaks Semantic MediaWiki
Closed, ResolvedPublic

Description

The 1.22.1 security release changed the visibility of WikiPage's $mPreparedEdit property from public to protected, which breaks Semantic MediaWiki (version 1.8.0.1), which appears to rely on that property being public. The exact error message is: PHP Fatal error: Cannot access protected property WikiPage::$mPreparedEdit in /extensions/SemanticMediaWiki/includes/SMW_ParseData.php on line 477

This is NOT acceptable, since this change has nothing to do with the security issues, which led into the release of a new 1.22.x version. Furthermore, a new WikiPage method called "clearPreparedEdit" was also introduced in the 1.22.1 release -- the documentation says "@since 1.23", which further proves that the WikiPage.php changes don't belong to the 1.22.x branch.


Version: 1.22.1
Severity: major

Details

Reference
bz60054

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 2:54 AM
bzimport set Reference to bz60054.
bzimport added a subscriber: Unknown Object (MLST).
Unknown Object (User) added a comment.Jan 14 2014, 10:20 PM

Just as a side note, the issue is relevant for all SMW1.9- legacy releases. SMW 1.9 did make all necessary adoptions in order to pass its unit tests (1.19/1.23, including the usage of prepareTextForEdit/prepareContentForEdit.

Will the MW 1.22.1 release be fixed and re-made? Will a 1.22.2 be created soonish? This compat break is rather unfortunate, and I'd rather not try to explain to users there are compat differences based on minor versions, it is hard enough as it is already.

https://gerrit.wikimedia.org/r/#/c/107488/ is a backport of the partial revert in core to the REL1_22 branch

I'm not sure how exactly should exactly it should be handled

For future releases, we should have some acceptance testing that verifies the visibility of member variables doesn't change on point releases.

Sam has a good method for handling this.

An eyeball test should be quite sufficient.

(In reply to comment #7)

An eyeball test should be quite sufficient.

Agreed. The problem with eyeball tests is that they cannot be automated and run reliably.

This was fixed in the 1.22.2 release.

So this bug in core was caused by a backport to stable of a patch for a bug which didn't have a request of backport and is not even against core (bug 57026)? I'm confused.
Anyway, https://gerrit.wikimedia.org/r/#/c/107488/ was merged.