⚓ T307691 JS pages show ToCs in vector-2022
Page MenuHomePhabricator
Create Task
Maniphest T307691

JS pages show ToCs in vector-2022
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To
None
Authored By
TheDJ
May 5 2022, 2:00 PM
Tags
Referenced Files
F35404491: Screen Shot 2022-08-06 at 3.53.26 PM.png
Aug 6 2022, 10:54 PM
F35404487: Screen Shot 2022-08-06 at 3.48.54 PM.png
Aug 6 2022, 10:50 PM
F35104862: Screenshot 2022-05-05 at 15.56.54.png
May 5 2022, 2:00 PM
Subscribers
Aklapper
cscott
daniel
Edtadros
Jdlrobson
Krinkle
ovasileva
TheDJ

Description

Page: https://en.wikipedia.org/wiki/MediaWiki:Gadget-popups.js

This contains raw <h2> elements. Because we parse JS pages in wikitext mode (for backlink support and categorization), but render them without wikitext mode, this page does actually have a ToC. It just never used to be visible until now.

Screenshot 2022-05-05 at 15.56.54.png (428×717 px, 82 KB)

Not sure what the best strategy for a fix here is, modify the script, or think about if we maybe should not present ToCs for certain content models (or both).

QA steps

QA Results - Beta

ACStatusDetails
1✅T307691#8135559

QA Results - Prod

ACStatusDetails
1✅T307691#8135562

Details

SubjectRepoBranchLines +/-
WikiPage: remove ::suppressTOC hackmediawiki/coremaster+13 -16
Use canonical parser options when rendering JavaScript/CSS for side effectsmediawiki/coremaster+164 -7
ParserOption/ParserOutput flag to suppress or hide the table of contentsmediawiki/coremaster+255 -20
Use ParserOptions::setSuppressTOC() for Javascript contentmediawiki/coremaster+25 -2
Certain content handlers should disable table of contents generationmediawiki/coremaster+26 -0
Do not transfer sections to OutputPage for site configmediawiki/coremaster+109 -7
Use ParserOptions::setSuppressTOC() for Javascript content (option 1)mediawiki/coremaster+5 -1
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedJdlrobsonT309972 [Goal] prepare desktop improvements project for further deployment
 ResolvedBUG REPORTNoneT307691 JS pages show ToCs in vector-2022

Event Timeline

TheDJ created this task.May 5 2022, 2:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 5 2022, 2:00 PM
Jdlrobson added a project: Web-Team-Backlog.May 11 2022, 3:18 PM
Jdlrobson subscribed.

Clear bug :)

Jdlrobson moved this task from Incoming to Table of Contents on the Desktop Improvements (Vector 2022) board.May 11 2022, 5:36 PM
Jdlrobson moved this task from Incoming to Not ready to estimate on the Web-Team-Backlog board.May 11 2022, 10:24 PM
ovasileva triaged this task as Medium priority.May 12 2022, 8:41 AM
ovasileva edited projects, added Web-Team-Backlog (Kanbanana-FY-2021-22); removed Web-Team-Backlog.
gerritbot added a comment.May 13 2022, 6:59 PM

Change 791659 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Do not transfer sections to OutputPage for site config

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

gerritbot added a project: Patch-For-Review.May 13 2022, 6:59 PM
Jdlrobson assigned this task to cscott.EditedMay 13 2022, 6:59 PM
Jdlrobson added projects: MediaWiki-Parser, Content-Transform-Team.
Jdlrobson moved this task from Needs Analysis to Blocked on Others on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
Jdlrobson added a subscriber: cscott.

@cscott I think this is a bug in parser/Article handling. Could I please request a review on the above patch?

cscott added a subscriber: Krinkle.May 16 2022, 3:45 PM

This is an interesting issue.

The "right" way to handle this is to set a flag in the ParserOutput to say "don't render the TOC". But we didn't actually do it that way, what we did was to elect "not to include the TOC marker element" in that case, and then rely on the substitution to find no match when it tries to insert the TOC -- and explicitly rely on the theme otherwise.

The Gadget-popups.js page could include:

// __NOTOC__

to work around this problem. But I think it would be better to suppress setSections() pre-emptively on "code type content", as @Krinkle suggests in his review on the above patch. In theory that could be done by the appropriate Content class calling:

$parserOutput->setPageProperty('notoc', '');

but unfortunately Parser only writes this page property, it doesn't read it. So I think probably a flag needs to be added to ParserOptions, so that code-type Content can set this when parsing.

Then the Parser will use its standard "notoc" handling and not set the section data on ParserOutput, which means that it won't get passed to OutputPage, and we'll have fixed this problem without an explicit hack in OutputPage.

Separately, we might want to indicate "no toc" status explicitly as a ParserOutput flag (instead of implicitly through "lack of a marker in the HTML") so that the skin can more carefully distinguish between "TOC is active, but no entries exist" and "TOC deliberately suppressed for this content type or by user request". But this latter task isn't a blocker.

cscott mentioned this in T249361: Is there a better way for Translate to disable PST on messages?.May 16 2022, 7:48 PM
gerritbot added a comment.May 19 2022, 8:01 PM

Change 793537 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Certain content handlers should disable table of contents generation

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

gerritbot added a comment.May 19 2022, 9:15 PM

Change 793539 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Define background size on logos with 1x and icon keys

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

gerritbot added a comment.May 24 2022, 11:37 PM

Change 791659 abandoned by Jdlrobson:

[mediawiki/core@master] Do not transfer sections to OutputPage for site config

Reason:

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

cscott added a comment.May 27 2022, 4:00 PM

Could someone clarify the code path here? We're rendering as Wikitext and generating a ParserOutput for category/backlink/etc purposes, then throwing away the generated HTML and rendering the code as preformatted text (or some such) instead?

gerritbot added a comment.May 27 2022, 4:21 PM

Change 800765 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Add ParserOption to suppress TOC when parsing non-wikitext pages

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

ovasileva edited projects, added Web-Team-Backlog; removed Web-Team-Backlog (Kanbanana-FY-2021-22).May 31 2022, 5:16 PM
ovasileva moved this task from Not ready to estimate to Tracking on the Web-Team-Backlog board.
cscott mentioned this in T310083: ToC issues.Jun 7 2022, 3:53 PM
gerritbot added a comment.Jun 15 2022, 7:25 PM

Change 805897 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Use ParserOptions::setSuppressTOC() for Javascript content

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

cscott mentioned this in T307251: [ToC] Show new/modified sections after publishing an edit (new floating ToC).Jun 15 2022, 9:13 PM
ovasileva subscribed.Jun 16 2022, 8:54 AM
gerritbot added a comment.Jun 16 2022, 2:14 PM

Change 806228 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Use ParserOptions::setSuppressTOC() for Javascript content (option 2)

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

gerritbot added a comment.Jun 16 2022, 2:14 PM

Change 806229 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Use canonical parser options when rendering Javascript pages for side effects

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

gerritbot added a comment.Jun 17 2022, 7:02 PM

Change 805897 abandoned by C. Scott Ananian:

[mediawiki/core@master] Use ParserOptions::setSuppressTOC() for Javascript content (option 1)

Reason:

Abandoned in favor of Iba6a8b6c59bf91e3d06896f0a610c3c3e52e6564

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

gerritbot added a comment.Jun 21 2022, 4:53 PM

Change 793537 abandoned by Jdlrobson:

[mediawiki/core@master] Certain content handlers should disable table of contents generation

Reason:

Abandoning in favor of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/806228

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

• JMcLeod_WMF removed a project: Content-Transform-Team.Jun 23 2022, 2:12 PM
Jdlrobson added a project: Content-Transform-Team.Jun 27 2022, 4:57 PM

My understanding is that web team is waiting on the content transform's patch https://gerrit.wikimedia.org/r/c/mediawiki/core/+/806228/ which is blocked on review by Daniel. Is this correct?

cscott added a comment.Jun 30 2022, 2:21 PM

Yes.

Jdlrobson reassigned this task from cscott to daniel.Jun 30 2022, 3:36 PM
• JMcLeod_WMF removed a project: Content-Transform-Team.Jul 7 2022, 2:08 PM
• JMcLeod_WMF subscribed.

Removed the content-transform-team tag; the Content Transform Team will deal with this under the MediaWiki-Parser project. It should be completed in the next week.

cscott mentioned this in T313455: Provide means for Content model to tweak ParserOptions.Jul 20 2022, 11:01 PM
cscott added a comment.Jul 21 2022, 7:18 PM

Waiting for a review from @daniel on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/800765 and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/806228 although we can probably C+2 this w/in the content transform team if we need to.

ovasileva added a parent task: T309972: [Goal] prepare desktop improvements project for further deployment.Jul 22 2022, 2:30 PM
gerritbot added a comment.Jul 27 2022, 8:24 AM

Change 800765 merged by jenkins-bot:

[mediawiki/core@master] ParserOption/ParserOutput flag to suppress or hide the table of contents

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

gerritbot added a comment.Jul 27 2022, 8:24 AM

Change 806228 merged by jenkins-bot:

[mediawiki/core@master] Use ParserOptions::setSuppressTOC() for Javascript content

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.23; 2022-08-01).Jul 27 2022, 9:00 AM
Jdlrobson edited projects, added Web-Team-Backlog (Kanbanana-2022-23-Q1); removed Web-Team-Backlog.Jul 27 2022, 6:54 PM

Thanks for getting these fixed. Adding to our sprint board for QA!

Jdlrobson moved this task from Needs Analysis to QA on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.Jul 27 2022, 6:54 PM
gerritbot added a comment.Jul 28 2022, 7:51 PM

Change 806229 merged by jenkins-bot:

[mediawiki/core@master] Use canonical parser options when rendering JavaScript/CSS for side effects

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

Jdlrobson updated the task description. (Show Details)Aug 4 2022, 7:45 AM
Edtadros moved this task from QA to QA in Prod on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.EditedAug 6 2022, 10:50 PM
Edtadros subscribed.

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: Beta cluster: Confirm https://en.wikipedia.beta.wmflabs.org/wiki/MediaWiki:Gadget-popups.js has no table of contents

Screen Shot 2022-08-06 at 3.48.54 PM.png (1×2 px, 539 KB)

Edtadros reassigned this task from daniel to ovasileva.Aug 6 2022, 10:54 PM
Edtadros moved this task from QA in Prod to Ready for Signoff on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.
Edtadros added a subscriber: daniel.

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

QA steps

AC1: Production - confirm https://en.wikipedia.org/wiki/MediaWiki:Gadget-popups.js has no table of contents

Screen Shot 2022-08-06 at 3.53.26 PM.png (1×2 px, 607 KB)

Edtadros reassigned this task from ovasileva to daniel.Aug 6 2022, 10:56 PM

oops...I'm used to assigning it when I move the column. Just assigning this back.

Edtadros updated the task description. (Show Details)Aug 6 2022, 11:02 PM
Jdlrobson closed this task as Resolved.Aug 15 2022, 11:09 AM
Jdlrobson removed daniel as the assignee of this task.

This can be resolved

• JMcLeod_WMF unsubscribed.Mar 30 2023, 1:28 PM
gerritbot added a comment.Nov 15 2023, 6:07 PM

Change 974657 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] WikiPage: remove ::suppressTOC hack

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

gerritbot added a comment.Feb 10 2024, 2:02 AM

Change 974657 merged by jenkins-bot:

[mediawiki/core@master] WikiPage: remove ::suppressTOC hack

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

Awesome_Aasim mentioned this in T373834: Don't parse templates and wikilinks outside of comments on CSS/JS pages.Sep 3 2024, 6:09 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 · Credits