ThreadItem and mw.dt.init are part of DiscussionTools.

The exception occurs because one of the comments on the page contains a navbox (it's inside this collapsed section: https://fr.wikipedia.org/wiki/Discussion:Léonard_de_Vinci#information_douteuse so I can't link to it directly – search the HTML source for "c-Crijam-2021-05-18T10:38:00.000Z-Relecture_partie_par_partie"), and the mobile transformations delete it, making DiscussionTools confused about where the comment starts.

We have two general directions we can go:

1. Accept that DOM corruption may happen, and live with it:
   • Replace the missing strart/end node with a best guess. This could break things like comment highlighting or future features that rely on the JS knowing the comment range.
   • Remove corrupted comments from the comment tree. This would disable the all reply tools features for such comments (e.g. the reply button)
2. Try to fix any sources of corruption:
   • Fix MobileFrontend to not hide navboxes in certain contexts, e.g. when under a heading (q: is it even possible for MF to detemine when a navbox is really is navbox and when it is part of a comment?)
   • As MF transformations happen after DT, tell MF to never remove nodes that contain comment markers, or to re-parent the comment markers
     • On the page in this example, the real navboxes contain a signature which is treated as a small comment, so they also will contain comment markers. This could perhaps be solved by telling MF to never remove partial comment markers (e.g. only a start marker).

The MF transformation in question seems directed in terms of product purpose at content pages, not discussion pages. So omitting it from that context more generally might be a good option. Depending on whether there are other MF transformations that do specifically relate to discussion pages, it might then be easier to skip it altogether or make it conditional on some value of "is this a content page" (we have a content namespaces concept in MW) or "is not a discussion page" (there are various different ways to approximate this, including newsectionlink and Signature namespaces, and DTs own utility method for this).

Thanks. It looks like the rule that's removing this box is defined in the config:

"MFRemovableClasses": {
	"value": {
		"beta": [],
		"base": [
			".navbox",
			".vertical-navbox",
			".nomobile"
		]
	}
}

In our WMF config we also add .mbox-image to this list.

Talk page yellow boxes are removed by CSS only (.tmbox). Hiding content with CSS doesn't break our tools as badly.

So some aditional sub-approaches to (2) would be;

• Reduce the number of elements removed by MFRemovableClasses (probably just to .nomobile) and replace navbox removal with CSS hides
• As Timo suggests, teach MF to know when DT is enabled, and disable some/all of these rules

I slightly favour the first of these as it reduces the amount of DOM manipulation that MF is doing, which seems like a good thing in the long run (even if it results in shipping a few more bytes per page), and there is already plenty of precedent for MF just hiding unwanted content with CSS (cf talk page yellow boxes).

It also avoids adding even more complexity to the MF transformations.

This looks like less of an edge case:

• https://en.m.wikipedia.org/wiki/Wikipedia:Village_pump_(proposals)?dtenable=1#Bot_proposal_(AFC_submission_templates)
• https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(proposals)#Bot_proposal_(AFC_submission_templates)

The thread has a header template https://en.wikipedia.org/wiki/Template:Rfc which is also stripped by MobileFrontend. which contains an .mbox-image which is stripped by MobileFrontend.

The CSS solution above would stop DT from crashing, but also in the situation the template should probably not be hidden at all.

During the discussion the Editing and Web Teams had today, we decided on doing the following:

• 1) @Esanders to confirm that no talk namespaces appear within the wgContentNamespaces config
• 2) Assuming the wgContentNamespaces config does NOT contain any talk namespaces, the Editing Team will:
  • A) Expose navboxes on all Wikimedia projects and talk pages while excluding:
    • Pages within the main namespace on all Wikipedias
    • Pages that exist within any namespace that is included in the wgContentNamespaces config on any non-Wikipedia project

to confirm that no talk namespaces appear within the wgContentNamespaces config

The following namespaces have wgExtraSignatures (and so have DiscussionTools) as well as being in wgContentNamespaces:

Main namespace on:

• incubatorwiki
• labswiki
• labtestwiki
• mediawikiwiki
• metawiki
• outreachwiki
• wikimania
• wikimedia
• private

Help namespace on:

• metawiki
• wikitext

Change 779042 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/MobileFrontend@master] Only apply MFRemovableClasses in content namespaces

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

@Esanders will editing handle code review for this one, or do you need web team input (if the latter please tag Web-Team-Backlog (Kanbanana-FY-2021-22) , if not I'll assume you have that all under control)?

We'll do it.

Change 779042 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Only apply MFRemovableClasses in content namespaces

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

Checked on the page mentioned on the task description, the navbox is now showing up on the mobile version of that talk page and also the error is gone from the console.

I also checked for this error on Logstash and it seems after this fix was deployed to all production wikis on Thursday, the number of occurrences for this error has drastically reduced.

~57 in last 4 days.
~4 in last 2 days.

@ppelberg : I am not sure I understand what you stated in the task description under "QA" section. The page you linked to does not contain any navbox and neither the talk page. Did you put a wrong link by any chance?

FYI, this caused a regression: T306705

(For future reference: that was actually caused by T219420)