Showing Table of contents sometimes requires reload (Vector 22, wmf.5)
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	Falloutgen
	Mar 31 2022, 7:50 AM

Description

Bonjour,

We are currently using WMF 5 (0aac639) and Vector 2022, since the activation of Table of contents, we notice that this one is not necessarily displayed at the first viewing of the page, and that it is sometimes necessary to refresh it so that it appears.

Once this is available, an additional shift is observed on the right side of the page where the two margins (right-left) are not equal:

See also our production site (activated by default): Fallout-wiki.com

Thanking you for your work,

Sincerely,

Kims

Details

	Subject	Repo	Branch	Lines +/-
	[Article] setSections must be called on non-cached parser output	mediawiki/core	master	+1 -0
	Skin::msg function should not have side effects on OutputPage	mediawiki/core	master	+33 -0

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Open		None	T276497 Scale DiscussionTools to all projects
Open		None	T366095 Deploy comment thanking to all wikis
Open		None	T310959 Scale Usability Improvements to all projects
Open		Trizek-WMF	T319146 [Release Ticket] Offer Usability Improvements as default-on feature at all projects (desktop)
Resolved		ppelberg	T302547 [Usability Improvements] Deploy as Beta Feature at all wikis (desktop)
Open		None	T249579 [EPIC] Usability improvements: make the actions, activity and content within talk pages easier to understand
Open		None	T269963 [EPIC] Talk Page Frame
Resolved		ppelberg	T304372 [Release ticket] Deploy Page Frame as Beta Feature at Partner Wikis
Resolved		matmarex	T317907 [Config Change] Enable Page Frame as beta feature at partner wikis (desktop)
Resolved		Ryasmeen	T303634 Adapt new ToC to help people understand and navigate pages that contain discussions
Resolved		ovasileva	T273473 [Epic] Improve the table of contents
Resolved		ovasileva	T304169 [Goal] Begin table of contents A/B test
Resolved	BUG REPORT	• nray	T305123 Showing Table of contents sometimes requires reload (Vector 22, wmf.5)

Event Timeline

Falloutgen created this task.Mar 31 2022, 7:50 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 31 2022, 7:50 AM

Aklapper renamed this task from Table of contents does not work properly (Vector 22, WMF 5) to Showing Table of contents sometimes requires reload (Vector 22, wmf.5).Mar 31 2022, 11:03 AM

Thanks for the report! We're investigating this issue in T303860.

(Also the feature is still in development so I don't recommend using it right now in a production setting!)

Moving to blocked on others, can check whether this is still occurring once T303860: ToC: ToC appears empty on certain pages is done

@ovasileva this is actually the issue I'm still investigating. I've closed the other one, as this describes the remaining issue perfectly.

Jdlrobson mentioned this in T303860: ToC: ToC appears empty on certain pages.Apr 1 2022, 3:17 PM

This is an issue in the MediaWiki parser that I'm still trying to debug.

I'm replicating it like follows:

Set:

$wgVectorTableOfContents = [
	'default' => false,
];

Load page
Set

$wgVectorTableOfContents = [
	'default' => true,
];

Load same page

For the 2nd load, it looks like the Parser calls internalParse with $isMain set to true and finalizeHeadings twice when the content is cached. The second call to finalizeHeadings results in an empty array of sections being stored.

Digging deeper, this seems to be triggered by the line:

$this->msg( 'retrievedfrom' )
			->rawParams( '<a dir="ltr" href="' . $url . '">' . $url . '</a>' )
			->parse();

That leads to setSections being called with empty sections array.

It looks like the message parser shares the OutputPage instance of Skin, so this seemingly harmless call.

I'm trying to dig out a minimum test case, but the expectation in Skin is that the OutputPage should refer to the page being outputted and frozen, not something that a call to message parsing can be modified. @cscott any idea what might be going on here?

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

[mediawiki/core@master] Skin::msg function should not have side effects on OutputPage

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

gerritbot added a project: Patch-For-Review.Apr 1 2022, 5:18 PM

ParserCache clearly has something to do with this. Perhaps setSections is not being called in this situation... I'm down a rabbit hole.

Jdlrobson removed Jdlrobson as the assignee of this task.Apr 1 2022, 7:00 PM

Jdlrobson moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.

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

[mediawiki/core@master] [Article] setSections must be called on non-cached parser output

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

Change 776239 abandoned by Jdlrobson:

[mediawiki/core@master] Skin::msg function should not have side effects on OutputPage

Reason:

I think I've got to the bottom of this.

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

In T305123#7823198, @Jdlrobson wrote:

Thanks for the report! We're investigating this issue in T303860.

(Also the feature is still in development so I don't recommend using it right now in a production setting!)

Hello,

Sorry I didn't see your answer, we have indeed disabled the feature, we had activated it to see if logs came up to help debugging :)

Sincerely,

Kims

ovasileva added a parent task: T304169: [Goal] Begin table of contents A/B test.Apr 4 2022, 5:39 PM

LGoto assigned this task to bwang.Apr 4 2022, 5:39 PM

Change 776245 merged by jenkins-bot:

[mediawiki/core@master] [Article] setSections must be called on non-cached parser output

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

Maintenance_bot removed a project: Patch-For-Review.Apr 4 2022, 8:31 PM

@nray since this is tricky to test on beta cluster, could you please verify the fix locally and sign this off?

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.6; 2022-04-04).Apr 4 2022, 9:00 PM

This appears to be working as intended. I was able to replicate the issue locally on the parent commit of the patch that got merged but was not able to replicate it with the patch. Therefore, I am resolving 😀

Jdlrobson mentioned this in T305576: [SPIKE] Define work to be done before discussion-specific ToC is production ready.Apr 7 2022, 4:37 PM

ppelberg added a parent task: T303634: Adapt new ToC to help people understand and navigate pages that contain discussions.Apr 7 2022, 10:58 PM

matmarex mentioned this in T316947: FlaggedRevisions incorrectly displays table of contents on Vector 2022 for flagged pages.Sep 7 2022, 8:01 PM

Jdlrobson mentioned this in T317333: addParserOutput should consult skin and call setSections when setting options relating to table of contents.Sep 8 2022, 3:51 PM

Showing Table of contents sometimes requires reload (Vector 22, wmf.5)Closed, ResolvedPublicBUG REPORTActions

Description

Details

Related ObjectsSearch...

Event Timeline

Showing Table of contents sometimes requires reload (Vector 22, wmf.5)
Closed, ResolvedPublicBUG REPORT
Actions

Related Objects
Search...