Page MenuHomePhabricator
Create Task
Maniphest T395196

Use ParserOptions::isMessage() instead of ParserOptions::getInterfaceMessage() in DiscussionTools
Closed, ResolvedPublic
Actions

Description

In theory, ParserOptions::isMessage() is an improved version of ParserOptions::getInterfaceMessage(), allowing us to detect when a message is being parsed in any language, not just user interface language.

In practice, trying to make this change caused T395034: Incorrect and doubled empty talk page onboard messages on every talk page of Chinese Wikipedia.

To do:

  1. Add a regression test for that bug
  2. Find out why that happened and fix the underlying problem
  3. Reapply https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/1143704

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Add tests for empty state hooksmediawiki/extensions/DiscussionToolsmaster+86 -0
OutputPage: Use ParserOptions::setIsMessage() in wrapWikiMsg()mediawiki/coremaster+11 -1
Revert^2 "In ParserAfterTidy use the new ParserOptions::isMessage"mediawiki/extensions/DiscussionToolsmaster+1 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

matmarex created this task.May 24 2025, 8:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 24 2025, 8:50 PM
matmarex mentioned this in T395034: Incorrect and doubled empty talk page onboard messages on every talk page of Chinese Wikipedia.May 24 2025, 8:52 PM
matmarex added a project: Technical-Debt.
Nikerabbit subscribed.May 26 2025, 8:38 AM
Tacsipacsi subscribed.May 26 2025, 6:47 PM
  1. Find out why that happened

rMW includes/Output/OutputPage.php:2050 (at d04dd77179d5) suspiciously sets only interfaceMessage, not message. And surprise, surprise, it does get called (indirectly) by rMW includes/page/Article.php:1292 (at d04dd77179d5).

and fix the underlying problem

This is a good question.

  • Should OutputPage::internalParserOptions() call both methods? It sounds like it’s none of its business.
  • Should ParserOptions::setInterfaceMessage(true) imply ParserOptions::setIsMessage(true) (of course without false implying false)? Sounds better in terms of responsibilities, but it may break other code.
  • Something else?
Jdforrester-WMF subscribed.May 27 2025, 5:54 PM
matmarex added a comment.May 29 2025, 2:19 PM
In T395196#10857607, @Tacsipacsi wrote:

rMW includes/Output/OutputPage.php:2050 (at d04dd77179d5) suspiciously sets only interfaceMessage, not message. And surprise, surprise, it does get called (indirectly) by rMW includes/page/Article.php:1292 (at d04dd77179d5).

(…)

  • Should OutputPage::internalParserOptions() call both methods? It sounds like it’s none of its business.
  • Should ParserOptions::setInterfaceMessage(true) imply ParserOptions::setIsMessage(true) (of course without false implying false)? Sounds better in terms of responsibilities, but it may break other code.
  • Something else?

Right. So anything that uses OutputPage::wrapWikiMsg (and it's a fairly common method) is going to have the interfaceMessage option set, but not the message option.

OutputPage::internalParserOptions is probably used for non-message wikitext parsing too, so we can't change it there, but we should probably change it for wrapWikiMsg, somehow.

We should probably review all uses of ParserOptions::setInterfaceMessage (https://codesearch.wmcloud.org/search/?q=setInterfaceMessage\b) and consider if they should use setIsMessage too. This would have caught T395023 too. I'm not sure if it would be good to make it implied.

matmarex mentioned this in T395589: Review uses of ParserOptions::setInterfaceMessage(), add setIsMessage() to them if appropriate.May 29 2025, 4:45 PM
matmarex added a subtask: T395589: Review uses of ParserOptions::setInterfaceMessage(), add setIsMessage() to them if appropriate.
gerritbot added a comment.May 29 2025, 6:36 PM

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

[mediawiki/core@master] OutputPage: Use ParserOptions::setIsMessage() in wrapWikiMsg()

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

gerritbot added a project: Patch-For-Review.May 29 2025, 6:36 PM

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

[mediawiki/extensions/DiscussionTools@master] Revert^2 "In ParserAfterTidy use the new ParserOptions::isMessage"

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

matmarex claimed this task.May 29 2025, 7:25 PM
gerritbot added a comment.May 29 2025, 8:20 PM

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

[mediawiki/extensions/DiscussionTools@master] Add tests for empty state hooks

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

gerritbot added a comment.May 30 2025, 2:51 AM

Change #1152123 merged by jenkins-bot:

[mediawiki/core@master] OutputPage: Use ParserOptions::setIsMessage() in wrapWikiMsg()

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

gerritbot added a comment.May 30 2025, 6:57 AM

Change #1152125 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Revert^2 "In ParserAfterTidy use the new ParserOptions::isMessage"

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

ReleaseTaggerBot added a project: MW-1.45-notes (1.45.0-wmf.4; 2025-06-03).May 30 2025, 7:00 AM
matmarex closed subtask T395589: Review uses of ParserOptions::setInterfaceMessage(), add setIsMessage() to them if appropriate as Resolved.Jun 5 2025, 12:26 AM
gerritbot added a comment.Jun 10 2025, 12:07 PM

Change #1152140 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Add tests for empty state hooks

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

Maintenance_bot removed a project: Patch-For-Review.Jun 10 2025, 12:30 PM
ReleaseTaggerBot edited projects, added MW-1.45-notes (1.45.0-wmf.6; 2025-06-17); removed MW-1.45-notes (1.45.0-wmf.4; 2025-06-03).Jun 10 2025, 1:00 PM
matmarex closed this task as Resolved.Jun 10 2025, 2:40 PM
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