Page MenuHomePhabricator

Metadata and buttons should be inserted after a heading, not inside of it
Open, Needs TriagePublicBUG REPORT

Description

Deployment plan

  • @matmarex to announce change on pl.wiki sometime this week
  • Tuesday, 29 Nov: @matmarex to backport this change to pl.wiki on Tuesday, 29 Nov
  • Monday, 5 Dec: Group 0
  • TBD: Group 1
    • Pending Tech/News announcement
  • TBD: Group 2 minus en.wiki
  • TBD: en.wiki

For Tech/News readers

(Announced in https://meta.wikimedia.org/wiki/Tech/News/2022/50)

Comparison of the HTML output for reference is below. Note that the ext-discussiontools-init-section-bar element is only visible for users who have enabled the DiscussionTools beta feature, but it is always included in the HTML.

BEFORE
<h2 class="ext-discussiontools-init-section">
	<span class="ext-discussiontools-init-section-subscribeButton ..." ...>...</span> <!-- Only included if topic subscription is enabled -->
	<span class="ext-discussiontools-init-section-subscribe mw-editsection-like">...</span> <!-- Only included if topic subscription is enabled -->
	<span class="mw-headline" ...>
		Heading text
	</span>
	<span class="mw-editsection">...</span>
	<div class="ext-discussiontools-init-section-bar">
		...
	</div>
</h2>
AFTER
<div class="mw-heading mw-heading2 ext-discussiontools-init-section">
	<span class="ext-discussiontools-init-section-subscribeButton ..." ...>...</span> <!-- Only included if topic subscription is enabled -->
	<h2>
		<span class="ext-discussiontools-init-section-subscribe mw-editsection-like">...</span> <!-- Only included if topic subscription is enabled -->
		<span class="mw-headline" ...>
			Heading text
		</span>
		<span class="mw-editsection">...</span>
	</h2>
	<div class="ext-discussiontools-init-section-bar">
		...
	</div>
</div>

I saw a link to https://www.mediawiki.org/wiki/Talk:Reading/Web/Desktop_Improvements#"Beginning"_is_confusing and browsed to it anonymously. For me, the current markup that DiscussionTools exposes for heading metadata is this:

<h2 class="ext-discussiontools-init-section"><!--__DTSUBSCRIBEBUTTONDESKTOP__h-Sdkb-2022-04-27T22:42:00.000Z--><!--__DTSUBSCRIBELINK__h-Sdkb-2022-04-27T22:42:00.000Z--><span id=".22Beginning.22_is_confusing"></span><span class="mw-headline" id="&quot;Beginning&quot;_is_confusing" data-mw-comment="{&quot;type&quot;:&quot;heading&quot;,&quot;level&quot;:0,&quot;id&quot;:&quot;h-\&quot;Beginning\&quot;_is_confusing-2022-04-27T22:42:00.000Z&quot;,&quot;replies&quot;:[&quot;c-Sdkb-2022-04-27T22:42:00.000Z-\&quot;Beginning\&quot;_is_confusing&quot;,&quot;c-132.170.199.112-2022-06-22T19:32:00.000Z-\&quot;Beginning\&quot;_is_confusing&quot;],&quot;headingLevel&quot;:2,&quot;name&quot;:&quot;h-Sdkb-2022-04-27T22:42:00.000Z&quot;}"><span data-mw-comment-start="" id="h-&quot;Beginning&quot;_is_confusing-2022-04-27T22:42:00.000Z"></span>"Beginning" is confusing<span data-mw-comment-end="h-&quot;Beginning&quot;_is_confusing-2022-04-27T22:42:00.000Z"></span></span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Talk:Reading/Web/Desktop_Improvements&amp;action=edit&amp;section=11" title="Edit section: &quot;Beginning&quot; is confusing">edit</a><span class="mw-editsection-bracket">]</span></span><div class="ext-discussiontools-init-section-bar"><div class="ext-discussiontools-init-section-metadata"><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-timestampLabel">Latest comment: <a href="#c-OVasileva_(WMF)-2022-07-11T13:58:00.000Z-Sdkb-2022-07-06T17:16:00.000Z">26 days ago</a></span><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-commentCountLabel">9 comments</span><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-authorCountLabel">6 people in discussion</span></div><div class="ext-discussiontools-init-section-actions"><!--__DTSUBSCRIBEBUTTONMOBILE__h-Sdkb-2022-04-27T22:42:00.000Z--></div></div></h2>

Though you might not have known this, this markup makes it so every heading is being read out like this in assistive technologies:

"Beginning is confusing" [edit] Latest comment: 26 days ago 9 comments 6 people in discussion

While logged in at https://www.mediawiki.org/wiki/Talk:Reading/Web/Desktop_Improvements?dt-enable=1&safemode=1#"Beginning"_is_confusing it's even worse:

<h2 class="ext-discussiontools-init-section"><span class="ext-discussiontools-init-section-subscribeButton oo-ui-widget oo-ui-widget-enabled oo-ui-buttonElement oo-ui-buttonElement-frameless oo-ui-iconElement oo-ui-labelElement oo-ui-flaggedElement-progressive oo-ui-buttonWidget" id="ooui-php-11" data-ooui="" data-mw-subscribed="null"><a class="oo-ui-buttonElement-button" role="button" title="Subscribe to receive notifications about new comments." tabindex="0" rel="nofollow"><span class="oo-ui-iconElement-icon oo-ui-icon-bellOutline oo-ui-image-progressive"></span><span class="oo-ui-labelElement-label">Subscribe</span><span class="oo-ui-indicatorElement-indicator oo-ui-indicatorElement-noIndicator oo-ui-image-progressive"></span></a></span><span class="ext-discussiontools-init-section-subscribe mw-editsection-like"><span class="ext-discussiontools-init-section-subscribe-bracket">[</span><a href="" class="ext-discussiontools-init-section-subscribe-link" role="button" tabindex="0" title="Subscribe to receive notifications about new comments.">subscribe</a><span class="ext-discussiontools-init-section-subscribe-bracket">]</span></span><span id=".22Beginning.22_is_confusing"></span><span class="mw-headline" id="&quot;Beginning&quot;_is_confusing" data-mw-comment="{&quot;type&quot;:&quot;heading&quot;,&quot;level&quot;:0,&quot;id&quot;:&quot;h-\&quot;Beginning\&quot;_is_confusing-2022-04-27T22:42:00.000Z&quot;,&quot;replies&quot;:[&quot;c-Sdkb-2022-04-27T22:42:00.000Z-\&quot;Beginning\&quot;_is_confusing&quot;,&quot;c-132.170.199.112-2022-06-22T19:32:00.000Z-\&quot;Beginning\&quot;_is_confusing&quot;],&quot;headingLevel&quot;:2,&quot;name&quot;:&quot;h-Sdkb-2022-04-27T22:42:00.000Z&quot;}"><span data-mw-comment-start="" id="h-&quot;Beginning&quot;_is_confusing-2022-04-27T22:42:00.000Z"></span>"Beginning" is confusing<span data-mw-comment-end="h-&quot;Beginning&quot;_is_confusing-2022-04-27T22:42:00.000Z"></span></span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/w/index.php?title=Talk:Reading/Web/Desktop_Improvements&amp;action=edit&amp;section=11" title="Edit section: &quot;Beginning&quot; is confusing">edit</a><span class="mw-editsection-bracket">]</span></span><div class="ext-discussiontools-init-section-bar"><div class="ext-discussiontools-init-section-metadata"><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-timestampLabel">Latest comment: <a href="#c-OVasileva_(WMF)-2022-07-11T13:58:00.000Z-Sdkb-2022-07-06T17:16:00.000Z">26 days ago</a></span><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-commentCountLabel">9 comments</span><span class="ext-discussiontools-init-section-metaitem ext-discussiontools-init-section-authorCountLabel">6 people in discussion</span></div><div class="ext-discussiontools-init-section-actions"></div></div></h2>

which resolves to

Subscribe \"Beginning\" is confusing​[edit] Latest comment: 26 days ago 9 comments 6 people in discussion

While some metadata might not seem as too much on one heading, it would certainly become a lot if every heading on the page is like this. Many users of assistive technologies use a list of headings to navigate the page, so they should be clear and concise (WCAG 2.1 criterion 2.4.6) about what each section represents. Usually, discussion authors try to keep them short and indicative of what a discussion is about. A bunch of metadata does not belong in every heading that would be exposed to assistive technologies. In addition, I was pointed by @Jack_who_built_the_house to MDN that says that heading elements are only supposed to have ‘Phrasing content’, which does not include <divs>.

If you want to make this a unified block, wrapping it in a <div> wrapper block instead would be better, as well as trying to fix whatever problems that have caused this usage in the first place. Abusing heading markup like this is a bad precedent.


This issue emerged in a conversation at en.wiki here: https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#c-Whatamidoing_(WMF)-20220913225300-DiscussionTools_Beta_Feature_update .

Event Timeline

We tried… It was changed back to the current state in rEDTOa3f665e81636: Remove <header> tags around headings for compat with MobileFrontend, because it didn't work with the collapsible sections in MobileFrontend. I was very sad about that.

Note that the same issue is present in MediaWiki core: T13555: .mw-editsection links should not be part of the <h#> element so I don't think our bug is the precedent (although it does make the situation worse).

@matmarex ‘Worse’ is kind of an understatement here. 'Edit' is one word, however annoying, in most languages (though there's also the fact that we put two links in most wikis, so...), DiscussionTools adds 12 minimum both before and after the heading. Whatever issues there are with MobileFrontend probably don’t justify this. To me, in the ideal world, this is where developers should step in and say to designers ‘this cannot be done because of technical reasons without harm’, in DiscussionTools’s case that applies to, I guess, mostly just to ‘Subscribe’ button being at one level with the heading and showing the info about the collapsed discussions on mobile, if MobileFrontend does not allow wrappers around heading elements.

ppelberg moved this task from Untriaged to Upcoming on the Editing-team board.
ppelberg moved this task from Backlog to Triaged on the DiscussionTools board.

@stjn can you please give the below a read and let me know if there is anything I've omitted/misunderstood about the experience people who use screen readers and other assistive technologies will experience as a result of this issue?

Behavior

  1. On wiki where you have the Show discussion activity setting in Special:Preferences enabled, visit a talk page. E.g. https://fr.wikipedia.org/wiki/Discussion:Citron .
  2. Ensure the screen reader you are using (e.g. en:JAWS) is also enabled
  3. Initiate the screen reader you enabled in "Step 2."

Actual behavior

  1. ❗️Notice as the screen reader reads section headings with metadata beneath them (e.g. the sections titled Huile de citron and Perte de vitamine...), it speaks the following text: Huile de citron edit source latest comment eleven years ago one comment one person in discussion

Expected behavior

  1. ✅ Notice as the screen reader reads section headings with metadata beneath them (e.g. the sections titled Huile de citron and Perte de vitamine...), it speaks the following text: Huile de citron.

(Given what you’ve said on VPT) I think merging [edit source] issue and DiscussionTools issue together is somewhat unnecessary. It should at least read Huile de citron [edit source], the underlying issue here is that DiscussionTools, to work properly with MobileFrontend (?), adds too much stuff into the headings. Fixing both this issue and T13555 at the same time would probably involve too much effort and negotiation (since edit links themselves are a really prominent element of the interface), but there’s no reason why DiscussionTools itself cannot use a more reasonable markup that would have some support from MobileFrontend.

"because it didn't work with the collapsible sections in MobileFrontend"

I don't see discussion activity in MobileFrontend anywhere, period. I can see it on desktop, tried with and without mobile advanced mode. Not that I want it on mobile, I don't want the stuff period. But if it never shows on MF anyway, why care about MF? .skin-minerva .ext-discussiontools-init-section-bar{display:none !important problem solved.

Also,

if ( $('.ext-discussiontools-init-section-bar')[0] ) {
insertAfter = function(element,newElement) {
	if ( element.nextElementSibling ) {
		element.parentElement.insertBefore(newElement,element.nextElementSibling);
	} else {
		element.parentElement.append(newElement);
	}
};
mw.util.addCSS('.ext-discussiontools-init-section-bar{margin-top:0 !important}');
for ( foo=0;foo<$('.ext-discussiontools-init-section-bar').length;foo++) {
	insertAfter($('.ext-discussiontools-init-section-bar')[foo].parentElement,$('.ext-discussiontools-init-section-bar')[foo]);
}
}

@AlexisJazz It is available in the mobile version, but only enabled on MediaWiki.org at the moment, because we're still working on it. You can try it here: https://m.mediawiki.org/wiki/Talk:Talk_pages_project/Usability.

(That page is also a good place to leave comments about how much you hate the feature. Please don't derail tasks that have a clearly defined problem and solution.)

(Given what you’ve said on VPT) I think merging [edit source] issue and DiscussionTools issue together is somewhat unnecessary. It should at least read Huile de citron [edit source], the underlying issue here is that DiscussionTools, to work properly with MobileFrontend (?), adds too much stuff into the headings. Fixing both this issue and T13555 at the same time would probably involve too much effort and negotiation (since edit links themselves are a really prominent element of the interface), but there’s no reason why DiscussionTools itself cannot use a more reasonable markup that would have some support from MobileFrontend.

Fair point.

In some ways, fixing this and T13555 at once would be easier, if we got it perfectly the first time – we could just update core, DiscussionTools and MobileFrontend once. If we do it separately, we'll need to update DIscussionTools and MobileFrontend now, and then core, DIscussionTools and MobileFrontend again for T13555. We'll also need to have more transitional code when working on that change (since the HTML output from the parser is cached, all new CSS and JS has to work with both the old and new HTML, and old CSS and JS should also work with both the old and new HTML in case the change is reverted, and now we'll have more versions of HTML).

I also worry that if we don't use this opportunity to fix T13555, it will never be fixed, because it's difficult to get WMF to prioritize maintenance work…

But you're right that T13555 will probably cause more issues, and maybe it should be done separately from this small change to talk pages.

I guess we should try to use a markup that will also work for T13555 with no changes (other than removing the metadata bar). This will make future transitions easier, and maybe also help prove that it works and find some of those nebulous issues.

If we're going to work on this, I'd like to do it, I've been thinking about this problem since at least 2013 ;) (T13555#167220)

I put some notes on the other task (T13555#8257798 and below).

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

[mediawiki/extensions/MobileFrontend@master] Allow collapsible sections with DiscussionTools wrappers on headings

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

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

[mediawiki/extensions/DiscussionTools@master] Move visualenhancements metadata and some buttons outside of `<h2>`

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

Change 843595 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Allow collapsible sections with DiscussionTools wrappers on headings

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

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

[operations/mediawiki-config@master] Use legacy DiscussionTools heading markup except on beta cluster

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

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

[operations/mediawiki-config@master] Use new DiscussionTools heading markup on enwiki and plwiki

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

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

[operations/mediawiki-config@master] Use new DiscussionTools heading markup everywhere

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

Change 856550 merged by jenkins-bot:

[operations/mediawiki-config@master] Use legacy DiscussionTools heading markup except on beta cluster

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

Mentioned in SAL (#wikimedia-operations) [2022-11-14T14:14:24Z] <taavi@deploy1002> Started scap: Backport for [[gerrit:856550|Use legacy DiscussionTools heading markup except on beta cluster (T314714)]], [[gerrit:855744|ThreadItemStore: Handle race conditions when finding/inserting outside of transaction (T322701)]], [[gerrit:855745|persistRevisionThreadItems: Print time taken]]

Mentioned in SAL (#wikimedia-operations) [2022-11-14T14:14:44Z] <taavi@deploy1002> taavi and matmarex: Backport for [[gerrit:856550|Use legacy DiscussionTools heading markup except on beta cluster (T314714)]], [[gerrit:855744|ThreadItemStore: Handle race conditions when finding/inserting outside of transaction (T322701)]], [[gerrit:855745|persistRevisionThreadItems: Print time taken]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmn

Mentioned in SAL (#wikimedia-operations) [2022-11-14T14:20:39Z] <taavi@deploy1002> Finished scap: Backport for [[gerrit:856550|Use legacy DiscussionTools heading markup except on beta cluster (T314714)]], [[gerrit:855744|ThreadItemStore: Handle race conditions when finding/inserting outside of transaction (T322701)]], [[gerrit:855745|persistRevisionThreadItems: Print time taken]] (duration: 06m 14s)

Change 843596 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Move visualenhancements metadata and some buttons outside of `<h2>`

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

You can probably tell what my plan for rolling this out is from the patches above. I'll consult the timing with the team and post some notifications on the wikis before I proceed, probably next week.

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

[mediawiki/extensions/DiscussionTools@master] Remove 'DiscussionToolsLegacyHeadingMarkup' config option

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

I was asked not to deploy anything to enwiki while they're angry at the WMF, so I guess I need a different plan.

If anyone watching this task would like to volunteer their site to receive this change first, and then review gadgets, site styles etc. for any problems, I would be grateful.

Is this in reference to entire DT mobile experience or just this task? Because it would be weird if it’s the latter.

while they're angry at the WMF

Lol, when are they not ?

Is this in reference to entire DT mobile experience or just this task? Because it would be weird if it’s the latter.

Just this task. We're being weirdly careful because gadgets and stuff tend to depend on things like these, and it's problematic to revert the change once if has been enabled (the new version gets cached).

Lol, when are they not ?

I have also wondered why folks being angry at someone else in my organization trying to push an unpopular decision would mean that I can't do things, but such is life, I guess.

Just this task. We're being weirdly careful because gadgets and stuff tend to depend on things like these, and it's problematic to revert the change once if has been enabled (the new version gets cached).

So the fact that someone is mad at the WMF in English Wikipedia means that a future default feature would be rolled out in a bad state. Tragic.

*shrug* We'll get to a good state eventually. Also, quite optimistic of you to assume that we'll roll it out that soon ;)

Next steps

  • @matmarex to announce change on pl.wiki sometime this week
  • @matmarex to backport this change to pl.wiki on Tuesday, 29 Nov
  • @ppelberg to file meta-task for deployment plan. Something like the below...
    • Phase 1: pl.wiki
    • Phase 2: nearly all wikis
    • Phase 3: big wikis (e.g en.wiki, de.wiki, fr.wiki, etc.)

Change 856551 merged by jenkins-bot:

[operations/mediawiki-config@master] Use new DiscussionTools heading markup on plwiki

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

Mentioned in SAL (#wikimedia-operations) [2022-11-29T21:34:57Z] <urbanecm@deploy1002> Started scap: Backport for [[gerrit:856551|Use new DiscussionTools heading markup on plwiki (T314714)]]

Mentioned in SAL (#wikimedia-operations) [2022-11-29T21:36:03Z] <urbanecm@deploy1002> urbanecm and matmarex: Backport for [[gerrit:856551|Use new DiscussionTools heading markup on plwiki (T314714)]] synced to the testservers: mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-11-29T21:42:00Z] <urbanecm@deploy1002> Finished scap: Backport for [[gerrit:856551|Use new DiscussionTools heading markup on plwiki (T314714)]] (duration: 07m 02s)

Mentioned in SAL (#wikimedia-operations) [2022-11-29T21:46:03Z] <urbanecm@deploy1002> Started scap: Backport for [[gerrit:856551|Use new DiscussionTools heading markup on plwiki (T314714)]]

Mentioned in SAL (#wikimedia-operations) [2022-11-29T21:51:18Z] <urbanecm@deploy1002> Finished scap: Backport for [[gerrit:856551|Use new DiscussionTools heading markup on plwiki (T314714)]] (duration: 05m 15s)

Next steps

  • @ppelberg to add specifics about the following phases to task description
    • Today (5-Dec): Group 0
    • Thursday (8-Dec): Group 1
    • TBD: Group 2 minus en.wiki
    • TBD: en.wiki

Change 856552 merged by jenkins-bot:

[operations/mediawiki-config@master] Use new DiscussionTools heading markup on group0 wikis

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

Mentioned in SAL (#wikimedia-operations) [2022-12-05T21:07:07Z] <samtar@deploy1002> Started scap: Backport for [[gerrit:856552|Use new DiscussionTools heading markup on group0 wikis (T314714)]]

Mentioned in SAL (#wikimedia-operations) [2022-12-05T21:08:50Z] <samtar@deploy1002> samtar and matmarex: Backport for [[gerrit:856552|Use new DiscussionTools heading markup on group0 wikis (T314714)]] synced to the testservers: mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-12-05T21:17:03Z] <samtar@deploy1002> Finished scap: Backport for [[gerrit:856552|Use new DiscussionTools heading markup on group0 wikis (T314714)]] (duration: 09m 55s)

ppelberg updated the task description. (Show Details)