Page MenuHomePhabricator

MFE still have issues with Parsoid Read Views on talk pages (Discussion Tools) - Type Error: startMarker is null
Closed, ResolvedPublic3 Estimated Story PointsPRODUCTION ERROR

Description

  • Visit a page with discussion tools and parsoid enabled, for example: https://en.wikivoyage.org/wiki/Talk:Western_food_in_Asia?useparsoid=1
  • Scroll to the very bottom and click on "Mobile view"
  • Note that the Discussion Tools "information bar" is missing under each heading. For example, under the first heading in desktop view reads "Latest comment: 4 years ago | 8 comments | 6 people in discussion", but in mobile view it just says "Latest comment: 4 years ago".
  • Open the JavaScript console and note the uncaught "Type Error: startMarker is null" on the console.
  • Click on any of the "reply" buttons: nothing happens.
  • Click the "add topic" button: nothing happens.

With the patch the mobile and desktop views should display the same information under the heading and there should be no JavaScript error on the console. The Reply and Add Topic buttons should work. T376048#10268521 has a few other items you can test.

QA steps

Requirement

Ensure the Type Error: startMarker is null issue is resolved for talk pages in MobileFrontend with Parsoid enabled, and verify that the Discussion Tools (Reply, Subscribe, and Add Topic) function as expected.

BDD

Feature: Fix discussion tools on talk pages with Parsoid enabled  
  Scenario: No JavaScript errors  
    Given a user visits a talk page with Parsoid enabled  
    When the page is loaded  
    Then there should be no JavaScript errors in the console  

  Scenario: Buttons appear under expanded headings  
    Given a user expands a heading on a talk page with Parsoid enabled  
    When the section is expanded  
    Then the Reply, Subscribe, and Add Topic buttons should appear  

  Scenario: Buttons function correctly  
    Given a user clicks the Reply, Subscribe, or Add Topic buttons  
    When the action is initiated  
    Then the respective tool or functionality should work correctly

Test Steps

Test Case 1: Verify no JavaScript errors

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Open the browser’s JavaScript console.
  3. Scroll through the page to ensure all components load.
  4. Verify that no JavaScript errors (e.g., Type Error: startMarker is null) are present.
  5. AC1: No errors appear in the JavaScript console.

Test Case 2: Verify buttons appear under headings

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Scroll to a section heading and expand it.
  3. Verify that the Reply, Subscribe, and Add Topic buttons appear under the expanded heading.
  4. AC2: The Reply, Subscribe, and Add Topic buttons are visible under expanded headings.

Test Case 3: Verify buttons function correctly

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Scroll to a section heading and expand it.
  3. Click on the Reply button and verify that the reply box opens.
  4. Click on the Subscribe button and verify that the subscription status updates.
  5. Click on the Add Topic button and verify that the topic creation tool opens.
  6. AC3: Reply, Subscribe, and Add Topic buttons function as expected.

Local development notes

This bug requires that MobileFrontend code runs before DiscussionTools. The bug only occurs with the following order of loadExtension calls:

wfLoadExtension('MobileFrontend');
wfLoadExtension('DiscussionTools');

Related Objects

StatusSubtypeAssignedTask
OpenReleaseNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenFeatureNone
OpenNone
OpenNone
ResolvedBUG REPORTNone
OpenNone
ResolvedJdforrester-WMF
OpenNone
Openovasileva
OpenNone
ResolvedPRODUCTION ERRORJdlrobson-WMF

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change #1075635 merged by jenkins-bot:

[operations/mediawiki-config@master] Turn on mobile support for Parsoid Read Views (but not on talk pages)

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

Mentioned in SAL (#wikimedia-operations) [2024-10-10T13:20:59Z] <lucaswerkmeister-wmde@deploy2002> Started scap sync-world: Backport for [[gerrit:1075635|Turn on mobile support for Parsoid Read Views (but not on talk pages) (T269499 T376048)]], [[gerrit:1079274|Turn on Parsoid Selective Update metrics (take 2) (T371713 T376433)]]

Mentioned in SAL (#wikimedia-operations) [2024-10-10T13:23:01Z] <lucaswerkmeister-wmde@deploy2002> lucaswerkmeister-wmde, cscott: Backport for [[gerrit:1075635|Turn on mobile support for Parsoid Read Views (but not on talk pages) (T269499 T376048)]], [[gerrit:1079274|Turn on Parsoid Selective Update metrics (take 2) (T371713 T376433)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-10-10T13:37:09Z] <lucaswerkmeister-wmde@deploy2002> Finished scap sync-world: Backport for [[gerrit:1075635|Turn on mobile support for Parsoid Read Views (but not on talk pages) (T269499 T376048)]], [[gerrit:1079274|Turn on Parsoid Selective Update metrics (take 2) (T371713 T376433)]] (duration: 16m 09s)

Some comments from a meeting with the editing team:

  • Issues with MobileFrontEnd + Parsoid Read Views + Discussion Tools:
    • https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1075635 is the patch to turn on Parsoid Read Views + MFE but not on discussion tools pages (see Jon’s comment there)
    • Tracking bug https://phabricator.wikimedia.org/T376048
    • Log in and select Parsoid read views under Preferences -> editing (scroll all the way down)
    • CSA: https://en.m.wikivoyage.org/wiki/Talk:Main_Page looks ok to me?
      • David: throws a javascript error and none of the buttons work https://phabricator.wikimedia.org/T376048#10211130
      • Going through all the features:
        • Reply tool
        • Permalink stuff
        • Adding a new topic
        • Topic subscription in each section
        • Page-level subscription in the “more” section
        • Extra UX: “latest comment on the entire page” at top (page frame) (does not include all the information from desktop, by design)
        • Each H2 section should have “latest comment” (again, a subset of info from desktop)
          • “Subscribe” should be hidden on mobile until you expand the section
          • Overflow menu on the right which contains the edit button
          • Vertical lines between H2 sections
        • Everything should be collapsed by default on load
      • Probably “banner hoisting” stuff is broken and “collapse by default” is broken because the JS error is interrupting everything.
      • “Add topic” bottom-of-page banner is out of place with and without Parsoid, but it’s differently out of place with Parsoid.
      • https://en.m.wikivoyage.org/wiki/Talk:Destinations is also broken, and doesn’t seem to have a broken H2 like the main page talk does.
        • ES: start/end markers inside headings appear to get deleted (data-mw-comment-start/end)
        • Eg <span data-mw-comment-start="" id="h-New_page?-2013-01-18T10:46:00.000Z"></span> is being stripped, just inside the <h2>

Ok, looked into this further. This is a conflict between MFE and discussion tools, as expected, but looks like the bug is in MFE's sectionCollapsing.js file.

Parsoid generates the following HTML for a heading:

<section data-mw-section-id="1" id="mwAw">
 <h2 id="First_discussion">
  First discussion
 </h2>
 <p>This is a first comment....

Discussion Tools then transforms this to:

<section data-mw-section-id="1" id="mwAw">
 <div class="mw-heading mw-heading2 ext-discussiontools-init-section">
  <!--__DTSUBSCRIBEBUTTONDESKTOP__...-->
  <h2 id="First_discussion" data-mw-thread-id="h-First_discussion-20241126170200">
   <span data-mw-comment-start="" id="h-First_discussion-20241126170200"></span>
   First discussion
   <span data-mw-comment-end="h-First_discussion-20241126170200"></span>
  </h2>
  ...

The <!-- --> comment is stripped out by DiscussionTools and the HTML is sent to the browser.

The sectionCollapsing.js code in MFE looks like this (last changed in T374578):

	const headingWrappers = Array.from( container.querySelectorAll( '.mw-parser-output > section > .mw-heading' ) );
	headingWrappers.forEach( ( wrapper ) => {
		wrapper.classList.add( 'mf-collapsible-heading' );
		const heading = wrapper.firstElementChild;
                ...
		// Update the heading text to account for semantics of collapsing sections
		const headingText = document.createElement( 'span' );
		headingText.textContent = heading.textContent;
                ...
		// Replace contents of the heading element
		heading.innerHTML = '';
		heading.append( icon );
		heading.append( headingText );

This is using heading.textContent so it strips out the <span> tags that DiscussionTools added and results in:

<section data-mw-section-id="1" id="mwAw">
 <div class="mw-heading mw-heading2 ext-discussiontools-init-section mf-collapsible-heading">
  <h2 id="First_discussion" data-mw-thread-id="h-First_discussion-20241126170200">
    <span class="mf-icon mf-icon--small mf-collapsible-icon mf-icon-collapse" aria-hidden="true"></span>
    <span tabindex="0" role="button" aria-controls="mwCw" aria-expanded="false">First discussion</span>  
  </h2>

For the legacy parser this looks like:

<div class="mw-heading mw-heading2 ext-discussiontools-init-section section-heading collapsible-heading open-block">
 <span class="mf-icon mf-icon-expand mf-icon--small indicator mf-icon-rotate-flip"> </span>
 <h2 data-mw-thread-id="h-First_discussion-20241126170200">
  <span class="mw-headline" id="First_discussion" tabindex="0" role="button" aria-controls="content-collapsible-block-0" aria-expanded="true">
   <span data-mw-comment-start="" id="h-First_discussion-20241126170200"></span>
   First discussion
   <span data-mw-comment-end="h-First_discussion-20241126170200"></span>
  </span>
  <span class="mw-editsection">
   <!-- this is made invisible by CSS -->
  </span>
 </h2>

So I assume the correct thing is to put all of the contents (not just the textContents inside the <span> that ends up with role="button" here.

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

[mediawiki/extensions/MobileFrontend@master] Don't overwrite DiscussionTools <span> tags during section collapsing xform

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

Can you update the description with some replication steps, please?

Replication steps (copied to task description as well):

  • Visit a page with discussion tools and parsoid enabled, for example: https://en.wikivoyage.org/wiki/Talk:Western_food_in_Asia?useparsoid=1
  • Scroll to the very bottom and click on "Mobile view"
  • Note that the Discussion Tools "information bar" is missing under each heading. For example, under the first heading in desktop view reads "Latest comment: 4 years ago | 8 comments | 6 people in discussion", but in mobile view it just says "Latest comment: 4 years ago".
  • Open the JavaScript console and note the uncaught "Type Error: startMarker is null" on the console.
  • Click on any of the "reply" buttons: nothing happens.
  • Click the "add topic" button: nothing happens.

With the patch the mobile and desktop views should display the same information under the heading and there should be no JavaScript error on the console. The Reply and Add Topic buttons should work. T376048#10268521 has a few other items you can test.

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

[operations/mediawiki-config@master] Allow defaulting to Parsoid Read Views when MobileFrontEnd is active

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

Change #1098113 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Don't overwrite DiscussionTools <span> tags during section collapsing xform

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

Jdlrobson changed the task status from Open to In Progress.Dec 5 2024, 6:10 PM

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

[mediawiki/extensions/MobileFrontend@master] Don't overwrite DiscussionTools <span> tags during section collapsing xform

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

Jdlrobson triaged this task as Medium priority.Jan 6 2025, 6:52 PM

@Jdlrobson You have now filed 2 duplicate bug reports about this known problem. There is a patch proposed for it that is blocked by your team (you reverted it before the holiday break). Could you consider approving it instead, so the bug is resolved?

The web team is at an offsite this week. It has been marked it as "in progress" to make sure this is ready for our next sprint which begins on January 13. Our understanding is that Web and Content Transform agreed to revert it given very few people were going to be around in the last deploy window of last year and this felt like a risky change with that in mind.

The web team is juggling a lot of things right now. T383253 was raised as part of a bug triage following the process outlined in https://www.mediawiki.org/wiki/Reading/Web/Chore_list. We log issues where production errors are found, the intention of our triage is to capture issues rather than understand them. Thanks for linking it to this task so they are connected.

If you have an issue with this time frame or how the team is working please bring it up with my team in #talk-to-web

Jdlrobson renamed this task from MFE still have issues with Parsoid Read Views on talk pages (Discussion Tools) to MFE still have issues with Parsoid Read Views on talk pages (Discussion Tools) - Type Error: startMarker is null.Jan 9 2025, 9:37 PM
Restricted Application changed the subtype of this task from "Task" to "Production Error". · View Herald TranscriptJan 9 2025, 9:37 PM
SToyofuku-WMF set the point value for this task to 3.Jan 9 2025, 10:55 PM

3 points as we should ensure there's some test coverage accompanying the patch around the ID behavior

SToyofuku-WMF changed the task status from In Progress to Open.Jan 9 2025, 10:55 PM
Jdlrobson changed the task status from Open to In Progress.Jan 10 2025, 11:30 PM

The web team is juggling a lot of things right now. T383253 was raised as part of a bug triage following the process outlined in https://www.mediawiki.org/wiki/Reading/Web/Chore_list. We log issues where production errors are found, the intention of our triage is to capture issues rather than understand them. Thanks for linking it to this task so they are connected.

If you have an issue with this time frame or how the team is working please bring it up with my team in #talk-to-web

Thanks for the link, I didn't know that you process was documented, and I'm glad to see it's shared with the community. I suggested a few improvements that should help you lose less time on duplicates: diff 1, diff 2, diff 3. I dropped a note in #talk-to-web on Slack too, we can continue there if you have any questions.

SToyofuku-WMF changed the task status from In Progress to Open.Jan 15 2025, 12:04 AM

Change #1101102 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Don't overwrite DiscussionTools <span> tags during section collapsing xform

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

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

[mediawiki/extensions/MobileFrontend@master] Fixes regression: edit icon collapses section

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

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

[mediawiki/extensions/MobileFrontend@master] Section edit link should not collapse sections in Parsoid

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

Change #1113238 abandoned by Jdlrobson:

[mediawiki/extensions/MobileFrontend@master] Fixes regression: edit icon collapses section

Reason:

See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/1113242

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

Change #1113242 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Section edit link should not collapse sections in Parsoid

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

Edtadros subscribed.

Status: ✅ PASS / ❓Need More Info
Environment: Beta
OS: macOS
Browser: Chrome
Device: MS

Test Case 1: Verify no JavaScript errors

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Open the browser’s JavaScript console.
  3. Scroll through the page to ensure all components load.
  4. Verify that no JavaScript errors (e.g., Type Error: startMarker is null) are present.
  5. ✅ AC1: No errors appear in the JavaScript console.

screenshot 141.png (842×1 px, 88 KB)

Test Case 2: Verify buttons appear under headings

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Scroll to a section heading and expand it.
  3. Verify that the Reply, Subscribe, and Add Topic buttons appear under the expanded heading.
  4. ❓ AC2: The Reply, Subscribe, and Add Topic buttons are visible under expanded headings.

The Add Topic button is at the bottom, not under the expanded headings

screenshot 142.png (919×1 px, 148 KB)

Test Case 3: Verify buttons function correctly

  1. Visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Talk:Dog&useparsoid=1.
  2. Scroll to a section heading and expand it.
  3. Click on the Reply button and verify that the reply box opens.
  4. Click on the Subscribe button and verify that the subscription status updates.
  5. Click on the Add Topic button and verify that the topic creation tool opens.
  6. ✅ AC3: Reply, Subscribe, and Add Topic buttons function as expected.

screenshot 89.mov.gif (920×1 px, 298 KB)

screenshot 90.mov.gif (920×1 px, 545 KB)

screenshot 88.mov.gif (920×1 px, 431 KB)

The Add Topic button is at the bottom, not under the expanded headings

Sorry that's my mistake in the A/C. This one is a pass! Thanks Edward!

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

[operations/mediawiki-config@master] Where Parsoid Read Views are the default, use it for MFE as well

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

Change #1134107 merged by jenkins-bot:

[operations/mediawiki-config@master] Where Parsoid Read Views are the default, use it for MFE as well

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

Mentioned in SAL (#wikimedia-operations) [2025-04-07T13:05:15Z] <jforrester@deploy1003> Started scap sync-world: Backport for [[gerrit:1134106|Shift to Parsoid Fragment support v3 (T390420)]], [[gerrit:1134107|Where Parsoid Read Views are the default, use it for MFE as well (T376048 T374578)]]

Mentioned in SAL (#wikimedia-operations) [2025-04-07T13:10:35Z] <jforrester@deploy1003> jforrester, cscott: Backport for [[gerrit:1134106|Shift to Parsoid Fragment support v3 (T390420)]], [[gerrit:1134107|Where Parsoid Read Views are the default, use it for MFE as well (T376048 T374578)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2025-04-07T13:26:10Z] <jforrester@deploy1003> Finished scap sync-world: Backport for [[gerrit:1134106|Shift to Parsoid Fragment support v3 (T390420)]], [[gerrit:1134107|Where Parsoid Read Views are the default, use it for MFE as well (T376048 T374578)]] (duration: 20m 54s)