Page MenuHomePhabricator
Create Task
Maniphest T95261

ImagePage: OutputPage::setFileVersion not called in diff view
Closed, DeclinedPublic
Actions

Description

The 'ImagePage' calls the 'setFileVersion' of the current 'OutputPage' object only at the end of method 'openShowImage' [1]. But this method just gets called if the page is not in 'diff' view [2].

Why is this a problem?
In core there is afaik no use of the 'fileVersion' field of 'OutputPage'. But there are extensions that rely on this information. A famous example is the FlaggedRevs extension, which uses this field to populate the review HTML form with hidden fields [3]. The described behavior results in a bad parametrized HTML form and therefore in a broken functionality.

What is a possible soultion?
One could just move the call of 'OutputPage::setFileVersion' out of 'ImagePage::openShowImage' and into 'ImagePage::view'

[1] http://git.wikimedia.org/blob/mediawiki%2Fcore.git/251b345151a1ce517d9f06f9bfa18d520a6c7e5a/includes%2Fpage%2FImagePage.php#L621
[2] http://git.wikimedia.org/blob/mediawiki%2Fcore.git/251b345151a1ce517d9f06f9bfa18d520a6c7e5a/includes%2Fpage%2FImagePage.php#L153
[3] http://git.wikimedia.org/blob/mediawiki%2Fextensions%2FFlaggedRevs.git/0057d275a4903ebd8750bbeabee4b69affecf48b/frontend%2FFlaggablePageView.php#L1138

Details

SubjectRepoBranchLines +/-
Ensure OutputPage::setFileVersion() is called in diff viewmediawiki/coremaster+2 -1
Customize query in gerrit

Event Timeline

Osnard created this task.Apr 7 2015, 6:12 AM
Osnard raised the priority of this task from to Needs Triage.
Osnard updated the task description. (Show Details)
Osnard subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 7 2015, 6:12 AM
Krenair added a project: MediaWiki-extensions-General.Apr 7 2015, 6:14 AM
Krenair subscribed.
Aklapper triaged this task as Low priority.Apr 7 2015, 2:00 PM
Aklapper added a project: Technical-Debt.
Aklapper set Security to None.
Osnard added a comment.Aug 25 2016, 1:52 PM

Added a change set to gerrit. Just in case somebody is interested.

https://gerrit.wikimedia.org/r/306664

gerritbot subscribed.Dec 25 2016, 1:07 AM

Change 306664 had a related patch set uploaded (by Legoktm):
Ensure OutputPage::setFileVersion() is called in diff view

https://gerrit.wikimedia.org/r/306664

gerritbot added a project: Patch-For-Review.Dec 25 2016, 1:07 AM
gerritbot added a comment.Oct 2 2018, 6:26 AM

Change 306664 abandoned by Robert Vogel:
Ensure OutputPage::setFileVersion() is called in diff view

Reason:
Probably not relevant anymore.

https://gerrit.wikimedia.org/r/306664

Osnard closed this task as Declined.Oct 2 2018, 8:55 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL