Page MenuHomePhabricator
Create Task
Maniphest T388427

Make sure there are no transclusion bugs due to global state access when transcluding Special:AllEvents
Closed, ResolvedPublic
Actions

Description

When transcluding a special page that accesses the global state directly, things could break in the most surprising way due to how special page transclusion works, by replacing the main context of the request. For details, see T290706: Parser code for special page transclusion replaces main context.

This task encompasses investigation and rectification of any issues with Special:AllEvents related to these problems. A partial list of things to fix can be found in T385729#10602473:

Aside from what the patch already does: no OOUI (and this is going to be a problem), the Pager constructor needs a context object, our UserLinker needs to be migrated from Linker to the new UserLinkRenderer.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Convert UserLinker to core's UserLinkRenderermediawiki/extensions/CampaignEventsmaster+10 -4
Add context parameter to UserLinker methodmediawiki/extensions/CampaignEventsmaster+36 -7
EventsListPager: avoid using OOUI to generate simple HTMLmediawiki/extensions/CampaignEventsmaster+51 -40
Add context constructor parameter to EventsListPagermediawiki/extensions/CampaignEventsmaster+20 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

MHorsey-WMF created this task.Mar 10 2025, 3:44 PM
MHorsey-WMF removed a project: Epic.
Daimona renamed this task from Make sure there are no transclusion bugs related to OOUI (See: T290706) to Make sure there are no transclusion bugs due to global state access (See: T290706).Mar 10 2025, 7:10 PM
Daimona updated the task description. (Show Details)
Daimona mentioned this in T388451: Convert TextWithIconWidget to Codex.
Daimona added a subtask: T388451: Convert TextWithIconWidget to Codex.
Daimona mentioned this in T387794: Design: Develop small/medium/large versions of embeddable Collaboration List.Mar 17 2025, 6:10 PM
Daimona edited projects, added Connection-Team (Connection-Current-Sprint); removed Connection-Team.Mar 18 2025, 7:10 PM
Daimona moved this task from Upcoming / refining 💡 to Ready for development on the Connection-Team (Connection-Current-Sprint) board.
Daimona claimed this task.Mar 19 2025, 2:19 PM
Daimona moved this task from Ready for development to Development In Progress 💻 on the Connection-Team (Connection-Current-Sprint) board.
gerritbot added a comment.Mar 19 2025, 2:28 PM

Change #1129282 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add context constructor parameter to EventsListPager

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

gerritbot added a project: Patch-For-Review.Mar 19 2025, 2:28 PM
gerritbot added a comment.Mar 19 2025, 3:38 PM

Change #1129302 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add context parameter to UserLinker method

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

gerritbot added a comment.Mar 19 2025, 4:50 PM

Change #1129321 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Convert UserLinker to core's UserLinkRenderer

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

gerritbot added a comment.Mar 21 2025, 2:28 PM

Change #1130133 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] EventsListPager: avoid using OOUI to generate simple HTML

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

gerritbot added a comment.Mar 21 2025, 3:25 PM

Change #1129282 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add context constructor parameter to EventsListPager

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

ReleaseTaggerBot added a project: MW-1.44-notes (1.44.0-wmf.22; 2025-03-25).Mar 21 2025, 4:00 PM
gerritbot added a comment.Mar 21 2025, 4:34 PM

Change #1130133 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] EventsListPager: avoid using OOUI to generate simple HTML

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

Daimona added a comment.Mar 24 2025, 6:16 PM

I have reviewed this again by:

  • Applying r958540 to MW core (patch for T290706)
  • Applying r1129321 to CampaignEvents (tip of the work to remove global state access from Special:AllEvents)
  • Making AllEvents includable, excluding the wikiproject tab, and the filter form
  • Commenting out direct OOUI usage in TextWithIconWidget

The following error is logged for everyone:

  • The code to build the navigation bar, and also some direct Message::text() calls in the pager, reach the global state via low-level Message code (Message->text() -> ... -> MessageParser->getParserOptions()). The navigation bar can simply be omitted (we already said this, and it would redirect to the main special page anyway). For direct usage, nothing we can do.

And for logged-out and temp users only, we also have:

  • IndexPager::__construct calls UserOptionsManager->getOption to get the default result limit ('rclimit' preference); this is also done a second time from getLimitOffsetForUser. This eventually reaches the global state via UserOptionsManager->loadOriginalOptions() -> LanguageConverter->getDefaultVariant() -> LanguageConverter->getURLVariant() -> RequestContext::getMain(). Basically nothing we can do about it, and every other includable special page based on IndexPager has the same issue.
  • Header dates are in the user's preferred timezone, so we call UserOptionsManager->getOption() which again reaches the global state via getDefaultVariant. Like above, not much we can do about it.
  • Event times are formatted according to the user's preferred timezone and time format, which once again calls UserOptionsManager->getOption() etc.
  • Message::text() for unnamed users also reaches the global state via UserOptionsManager.
  • Session->__destruct() -> ... -> User->getName() -> User->getRequest(): nothing we can do.

Overall, this means that converting TextWithIconWidget is the last thing left to do for this task.

Daimona moved this task from Development In Progress 💻 to Code Review 💬 on the Connection-Team (Connection-Current-Sprint) board.Mar 25 2025, 10:48 PM
Daimona renamed this task from Make sure there are no transclusion bugs due to global state access (See: T290706) to Make sure there are no transclusion bugs due to global state access when transcluding Special:AllEvents.Mar 26 2025, 5:00 PM
Daimona mentioned this in T390084: Make sure there are no transclusion bugs due to global state access when transcluding the communities tab of Special:AllEvents.Mar 26 2025, 5:03 PM
gerritbot added a comment.Mar 28 2025, 9:05 PM

Change #1129302 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add context parameter to UserLinker method

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

gerritbot added a comment.Mar 28 2025, 9:05 PM

Change #1129321 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Convert UserLinker to core's UserLinkRenderer

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

Maintenance_bot removed a project: Patch-For-Review.Mar 28 2025, 9:30 PM
ReleaseTaggerBot edited projects, added MW-1.44-notes (1.44.0-wmf.23; 2025-04-01); removed MW-1.44-notes (1.44.0-wmf.22; 2025-03-25).Mar 28 2025, 10:00 PM
Daimona closed subtask T388451: Convert TextWithIconWidget to Codex as Resolved.Apr 1 2025, 3:06 PM
Daimona reopened subtask T388451: Convert TextWithIconWidget to Codex as Open.Apr 1 2025, 3:14 PM
Daimona moved this task from Code Review 💬 to QA 🐛 on the Connection-Team (Connection-Current-Sprint) board.Apr 3 2025, 1:16 PM

Note: this cannot be tested until we actually make the page transcludable (T388385).

MHorsey-WMF changed the status of subtask T388451: Convert TextWithIconWidget to Codex from Open to In Progress.Apr 10 2025, 3:00 PM
MHorsey-WMF changed the status of subtask T388451: Convert TextWithIconWidget to Codex from In Progress to Stalled.
ifried changed the task status from Open to Stalled.Apr 14 2025, 4:40 PM
ifried moved this task from QA 🐛 to Blocked ⛔ on the Connection-Team (Connection-Current-Sprint) board.
Daimona changed the status of subtask T388451: Convert TextWithIconWidget to Codex from Stalled to Open.Apr 15 2025, 4:26 PM
vaughnwalters closed subtask T388451: Convert TextWithIconWidget to Codex as Resolved.Apr 16 2025, 10:10 PM
VPuffetMichel added a project: OKR-Work.Apr 17 2025, 1:35 PM
Daimona changed the task status from Stalled to Open.Apr 17 2025, 5:50 PM
Daimona moved this task from Blocked ⛔ to QA 🐛 on the Connection-Team (Connection-Current-Sprint) board.
vaughnwalters moved this task from QA 🐛 to Product sign-off 🖌️ on the Connection-Team (Connection-Current-Sprint) board.May 5 2025, 9:54 PM
vaughnwalters subscribed.

Any additional bugs found and remaining issues are being dealt with in T388385 (or tickets created and linked from there) so I am sending this to product sign off

ifried closed this task as Resolved.May 19 2025, 8:59 PM
ifried moved this task from Product sign-off 🖌️ to Done 🏁 on the Connection-Team (Connection-Current-Sprint) board.
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