Page MenuHomePhabricator
Create Task
Maniphest T325562

Pull OutputPage::setSections($parserOutput->getSections()) into OutputPage::addParserOutputMetadata()
Closed, ResolvedPublic
Actions

Description

These method calls often occur together, ie from EditPage.php:

$out->setSections( $parserOutput->getSections() );
$out->addParserOutputMetadata( $parserOutput );

These can probably be combined, pulling the setSections call into addParserOutputMetadata.

See https://codesearch.wmcloud.org/deployed/?q=addParserOutputMetadata&i=nope&files=&excludeFiles=&repos=

Follow up to T317333#8223090

Details

SubjectRepoBranchLines +/-
OutputPage: Call setSections() from addParserOutputMetadata()mediawiki/coremaster+3 -7
Remove redundant calls to OutputPage::setSections()mediawiki/extensions/FlaggedRevsmaster+0 -2
Customize query in gerrit

Related Objects

Event Timeline

cscott created this task.Dec 19 2022, 4:23 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 19 2022, 4:23 PM
cscott mentioned this in T317333: addParserOutput should consult skin and call setSections when setting options relating to table of contents.Dec 19 2022, 4:51 PM
matmarex claimed this task.Dec 19 2022, 10:46 PM
matmarex subscribed.
In T317333#8223090, @cscott wrote:

The other one is that addParserOutputMetadata doesn't set the section data. So practically every call to OutputPage::addParserOutput() is preceded by a call to OutputPage::setSections( $parserOutput->getSections()). It seems like we should really fold that into addParserOutput... but I suspect there are probably cases where we *don't* want to overwrite the section data, so I'm going to split that refactor into a separate task.

I've reviewed the calls to addParserOutput...() in core, haven't found any where this change would be incorrect, and I found one where it actually fixes a bug (now filed as T325600).

gerritbot added a comment.Dec 19 2022, 10:48 PM

Change 869326 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] OutputPage: Call setSections() from addParserOutputMetadata()

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

gerritbot added a project: Patch-For-Review.Dec 19 2022, 10:48 PM
gerritbot added a comment.Dec 19 2022, 10:51 PM

Change 869328 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/FlaggedRevs@master] Remove redundant calls to OutputPage::setSections()

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

ovasileva subscribed.Dec 20 2022, 9:10 PM
gerritbot added a comment.Jan 16 2023, 10:23 PM

Change 869326 merged by jenkins-bot:

[mediawiki/core@master] OutputPage: Call setSections() from addParserOutputMetadata()

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

gerritbot added a comment.Jan 16 2023, 10:23 PM

Change 869328 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Remove redundant calls to OutputPage::setSections()

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

Maintenance_bot removed a project: Patch-For-Review.Jan 16 2023, 10:30 PM
ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.19; 2023-01-16).Jan 16 2023, 11:00 PM
matmarex closed this task as Resolved.Jan 16 2023, 11:33 PM
cscott added a comment.Jan 19 2023, 5:03 PM

Thanks so much for tackling this!

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 · Credits