Page MenuHomePhabricator

.mw-editsection links should not be part of the <h#> element
Open, MediumPublic

Description

Currently, screen reader users who navigate by headings constantly hear the word "edit" at the end of each heading because section edit links are part of the <h#> elements. It should be changed so that the <h#> elements contain only the heading text, with the section edit links separated out.

This was originally a bug for the link coming before the heading text, which has since been changed. The original description is preserved below:

Author: rene.kijewski

Description:
Proposed change

For users of screenreaders (and textbrowsers) it takes quite long to walk throught the headlines. At every single headline they hear "[edit]" first instead of the real section title. This could easily be changed by let the edit-section-link come second in the rendered page and let the section title come first. There would not even be visual changes for seeing users and users of graphical browsers.

Done

  • Editing Team to draft announcement for Tech/News that includes link to documentation explaining:
    • Reason for change
    • Change
    • Instructions for how to cope with change
    • Who is likely to be impacted by change

Details

Reference
bz11555
SubjectRepoBranchLines +/-
mediawiki/coremaster+455 -287
mediawiki/extensions/MobileFrontendmaster+3 -3
mediawiki/extensions/VisualEditormaster+6 -3
mediawiki/extensions/DiscussionToolsmaster+336 -338
mediawiki/extensions/PageImagesmaster+4 -6
mediawiki/extensions/Scribuntomaster+7 -5
mediawiki/extensions/DiscussionToolsmaster+704 -670
mediawiki/extensions/LabeledSectionTransclusionmaster+6 -6
mediawiki/coremaster+38 -39
mediawiki/coremaster+1 -0
mediawiki/coremaster+37 -39
mediawiki/coremaster+55 -2
mediawiki/extensions/DiscussionToolsmaster+14 -9
mediawiki/skins/Vectormaster+78 -6
mediawiki/skins/CologneBluemaster+4 -0
mediawiki/coremaster+35 -0
mediawiki/skins/Modernmaster+21 -0
mediawiki/skins/Nostalgiamaster+16 -0
mediawiki/skins/MinervaNeuemaster+62 -7
mediawiki/skins/Timelessmaster+34 -0
mediawiki/coremaster+71 -20
mediawiki/skins/Vectormaster+5 -3
mediawiki/skins/MinervaNeuemaster+4 -2
Show related patches Customize query in gerrit

Related Objects

Event Timeline

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

There may be Parsoid/VisualEditor changes needed as well. We'll need to decide whether to change the heading structure in "edit mode" HTML, or whether this is a transformation of the "editable" HTML which only applies to read views.

I have a patch up for VisualEditor that adjusts it for these changes: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/859139. Parsoid does not currently have section edit links, or any other "embellishments" on the headings, so I did not propose any changes for it.

When we will be adding them, I would prefer to make it a transformation that only applies to read views. I could work on that, but I thought that the necessary "framework" for doing that doesn't exist yet, although I know it's planned – if (or when) it exists, please point it out and I'll write some patches.

Proposed markup is, more-or-less:

<section>
 <div class="heading-stuff">
  <h2>Heading</h2>
  <div>[edit] (etc)</div>
 </div>
 <div class="content-stuff">
  Lorem ipsum...
 </div>
 <section>
    ... nested section...
 </section>
</section>

Currently thinking that this markup would be part of T269630: Parsoid should support section editing links and would only apply to read views, aka not affect edit mode, VE or the MediaWiki DOM spec. In that case, the parsoid-side implications are much reduced, since we haven't implemented T269630 yet in any case.

(forgot to send this)
From the recent web/ editing sync (12th September):

Web team was asked to weigh in on this ticket.

  • We are excited about this change and think it’s a good idea
  • We are concerned about the how
    • how this breaks 3rd party skins
    • We are concerned about how this might impact gadgets.
  • We are concerned about the number of patches involved in the change and would not feel comfortable with them going out in a single deploy
  • We are concerned that there’s no clear rollback plan that takes into account cached HTML.

Suggestion:

  • We think the change should be opt in (at least over 2 MediaWiki releases) and we should support both legacy and new format.
  • We did something similar for breaking table of contents out of the article (skin-specific parser flag)
  • We think we should roll this out on one skin first e.g. Minerva, and then other skins later on.
  • We think this work should be spread out over a month at least.

Happy to talk further if any of the suggestions need elaboration.

@Jdlrobson do you/your team have a strong feeling w/r/t the wrapper around section content (class="content-stuff" in T13555#8325916) to allow section collapsing? @matmarex didn't have a strong feeling about that, and parsing has a vague idea that folks might want that, but not strong enough to push hard for it.

@cscott the introduction of content-stuff would allow us to throw away a lot of code in MobileFrontend so yes I would be very much for that, if we're planning changes to the HTML any way. Ideally this would be initially an opt-in feature for skins similar to how we make table of contents behave differently depending on the skin.

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

[mediawiki/core@master] Add styles for new heading HTML

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

Change 975362 merged by jenkins-bot:

[mediawiki/core@master] Add styles for new heading HTML

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

Change 857767 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Add styles for new heading HTML

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

Change 857770 merged by jenkins-bot:

[mediawiki/skins/Timeless@master] Add styles for new heading HTML

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

Change 857769 merged by jenkins-bot:

[mediawiki/skins/Nostalgia@master] Add styles for new heading HTML

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

Change 857768 merged by jenkins-bot:

[mediawiki/skins/Modern@master] Add styles for new heading HTML

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

Change 857766 merged by jenkins-bot:

[mediawiki/skins/CologneBlue@master] Add styles for new heading HTML

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

Change 842860 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Add styles for new heading HTML

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

^ core patch has broken DT headings:

image.png (481×1 px, 80 KB)

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

[mediawiki/extensions/DiscussionTools@master] Compatibility with styles for new heading HTML

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

Change 979130 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Compatibility with styles for new heading HTML

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

Before this is unleashed, please could you post here the actual new heading HTML, so we can plan for any necessary user script changes.

@GhostInTheMachine Please see https://www.mediawiki.org/wiki/Heading_HTML_changes#What's_changing (some parts of that documentation page are still in progress, but this is agreed on).

@matmarex Will this be live and visible to users the week after next week (1.42.0-wmf.9), or will it still be behind a feature flag (per T13555#8422033 above) for a few more weeks?
Or in other words, when should it be included in Tech News, and what should the entry say and link to? Thanks!

It's not going to be live yet, the changes aren't ready. I'm not sure when it will happen, but I'll add an entry to Tech News when it does.

Since the update of 1.42.0-wmf.9 it seems like DiscussionTools already applies the heading html transformation - I had to hotfix dewiki's main page as it looked broken. You can see the broken look still on other pages like https://de.wikipedia.org/wiki/Wikipedia:Autorenportal or https://de.wikipedia.org/wiki/Hilfe:%C3%9Cbersicht. I'm also not sure why DiscussionTools is active on these page(s), though.

@hgzh DiscussionTools has used the new heading markup for more than a year, since T314714. It's active (in some ways – not all features are enabled everywhere) on all talk pages and pages in $wgExtraSignatureNamespaces, which includes Wikipedia: and Help: on most Wikimedia wikis, so the new markup has been active on the two pages you linked.

However, wmf.9 contains some styling changes (core / skin / DiscussionTools), which, in hindsight, could have disturbed custom styling. I'm sorry about that… I hope that this only affected a few pages. I can help fix the local styles if you'd like.

Thanks for clarification, then it's the additional styling causing the issues. We're discussing approaches for fixes at https://de.wikipedia.org/wiki/Wikipedia:Technik/Skin/MediaWiki/%C3%84nderungen#Ge%C3%A4nderte_%C3%9Cberschriftenstruktur but it doesn't seem so easy at first glance - maybe you have an idea, your input would be appreciated. For dewiki's main page I did this: https://de.wikipedia.org/w/index.php?title=Wikipedia%3AHauptseite%2F%21box&diff=240191659&oldid=213295348

Current status: I'm reworking some of the proposed patches, because folks requested to make it possible for skins to disable or enable this behavior. This turns out to be surprisingly difficult to do, because currently the headings don't depend on the skin, so allowing that requires either splitting the parser cache for each skin (increasing disk space usage), or moving some of the work to happen after the parse is retrieved from the parser cache. I'm working on the second approach.

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

[mediawiki/core@master] Move section heading formatting to post-cache transform

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

wmf.9 contains some styling changes (core / skin / DiscussionTools), which, in hindsight, could have disturbed custom styling.

I think several templateStyles sheets on Meta-Wiki are affected (example). Do you know a way to find all of them, so I can fix them?

Basic search finds about 130 pages potentially needing a change.

Hi, on frwiki homepage (permalink), I noticed there is an element in the wikicode that doesn't get converted to the new markup: <h2 style="margin-top:1.5rem">. When removing the style="margin-top:1.5rem", the element gets converted, as the other <h2>'s of the page.

Is it intentional that the markup change is avoided in this case? (I don't think so, as ultimately the old markup is expected to be found nowhere)

And how should MediaWiki handle such case (i.e. what to do with the style attribute)?

Hi, on frwiki homepage (permalink), I noticed there is an element in the wikicode that doesn't get converted to the new markup: <h2 style="margin-top:1.5rem">. When removing the style="margin-top:1.5rem", the element gets converted, as the other <h2>'s of the page.

Is it intentional that the markup change is avoided in this case? (I don't think so, as ultimately the old markup is expected to be found nowhere)

And how should MediaWiki handle such case (i.e. what to do with the style attribute)?

<h2> with style or class attributes do not get any wrapper since T353489. I agree it is confusing for templateStyles.


Also, in the future (after migration), will we keep .mw-heading2, h2 and opposite .mw-heading2 h2 ? We may want to separate rules in two sets instead (one for .mw-heading2 block, another one for inline h2). But will that work on special pages, or with Parsoid?

Instead of having to create mixed backward and forward compatible rules, I would prefer to have version-specific rulesets, e.g. switching from <body class="heading-v1"> to <body class="heading-v2">.

Another thing, here's the new markup I currently see on the pages:

<div class="mw-heading mw-heading2">
    <h2>
        <span id="..." class="mw-headline">...</span>
        <span class="mw-editsection">...</span>
    </h2>
</div>

I noticed there is a problem because the "edit section" element is not next to the <h2>, but inside it, hence missing the accessibility improvements...

That markup doesn't match the old or the new markup shown here, but is a mix of the two… (it seems to be just a <div class="mw-heading ..."> container wrapped around the old markup).

@Od1n it is because this task has not been completed. Only T314714 has been done, for DiscussionTools.
“mw:Heading HTML changes” show expected changes in the future.

I completed the review and fix for the 135 templateStyles sheet on Meta-Wiki which contain a h# selector.
In most cases, I could simply replace h2 with .mw-heading2, however there are two main issues:

  • color property is not inherited by h2.
  • If the style sheet is used both on Main namespace and on specific namespaces (e.g. Grant: or Research:), the inconsistency (wrapper or not) makes maintenance really painful.

I confirm that having these many markups, that overlap, makes it a headache to create selectors that pick the desired elements, and only these.

We need clear, non-ambiguous selection (not "oh hey let's grab everything and apply the CSS to all of that"), as the use cases are varying: we may want the id attribute (for anchors), target the <h2> (to format the text, e.g. font style or underlining), target the container (for margins)…

For instance, for a given heading we could erroneously select two elements, instead of only one.

I think the most important cause of trouble is that "hybrid markup".

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

[mediawiki/core@master] Move section heading formatting to post-cache transform (forward-compat)

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

Change 998533 merged by jenkins-bot:

[mediawiki/core@master] Move section heading formatting to post-cache transform (forward-compat)

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

Also, in the future (after migration), will we keep .mw-heading2, h2 and opposite .mw-heading2 h2 ? We may want to separate rules in two sets instead (one for .mw-heading2 block, another one for inline h2). But will that work on special pages, or with Parsoid?

Parsoid output for page views will be the same as the new markup for the legacy parser, with the .mw-heading wrappers. It's being implemented in task T269630, and should go live soon (possibly this month).

Parsoid output within the visual editor will not have the wrappers. Special pages will generally also just output <h2> headings with no wrappers.

I completed the review and fix for the 135 templateStyles sheet on Meta-Wiki which contain a h# selector.
In most cases, I could simply replace h2 with .mw-heading2, however there are two main issues:

  • color property is not inherited by h2. (…)

Thanks for doing that. I think it would make sense to inherit the color too, since it already inherits the font. I'll submit a patch.

(…) If the style sheet is used both on Main namespace and on specific namespaces (e.g. Grant: or Research:), the inconsistency (wrapper or not) makes maintenance really painful.

I confirm that having these many markups, that overlap, makes it a headache to create selectors that pick the desired elements, and only these.

We need clear, non-ambiguous selection (not "oh hey let's grab everything and apply the CSS to all of that"), as the use cases are varying: we may want the id attribute (for anchors), target the <h2> (to format the text, e.g. font style or underlining), target the container (for margins)…

For instance, for a given heading we could erroneously select two elements, instead of only one.

I think the most important cause of trouble is that "hybrid markup".

I'm sorry about that folks, when I started this project over a year ago (and made the changes for talk pages in T314714), I thought I'd be done in a month or two. I promise that we're making progress though.

The "hybrid markup" on talk pages will be replaced by the new markup. The "old markup" also will be replaced (permanently) by the new markup – they will only coexist for a short time, until everything is switched over. (At least I hope it will be a short time.)

That will only leave two cases to think about when writing TemplateStyles, which will coexist: new markup, and <hN> tags with no wrappers – let's call this "plain markup". You usually won't need to write styles for the "plain markup", unless you need your styles to apply to headings with manually added attributes in wikitext, or while using the visual editor (or if your style is supposed to apply on special pages, but that's weird).

I hope that makes sense, and I once again apologize for the current mess.

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

[mediawiki/core@master] Inherit text color for `hN` nested inside `.mw-heading`

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

Change 990744 merged by jenkins-bot:

[mediawiki/core@master] Move section heading formatting to post-cache transform

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

Removal of the "old" and "hybrid" markups will definitely improve the situation. I would have preferred to no longer see the "plain" markup too, but if converting an input such as <h2 style="color:orange; margin-top:2em;"> to "new markup", as the color isn't inherited, it would have to be directly applied to the inner <h2>, and the the margin-top would preferably have to be applied to the outer <div class="mw-heading">… of course, it is not feasible to implement handling for each and every property…

With only the "new" and the "bare" markups remaining, we can easily target all <h2>'s by simply using .h2 selector. If ever we need to select only the bare <h2>'s, we can do :not(.mw-heading) > h2. And to target all headings at the topmost level, we could do .mw-heading2, :not(.mw-heading) > h2.

While writing this, I'm getting a second thought. Actually, I think we should enclose the bare <h2>'s with a <div class="mw-heading mw-heading2">, and keep the provided attributes on the generated <h2>. It would unify the markup and let simplify the selectors. I can't think of cases it would harm, but maybe you have some? And if this enclosing is not possible (though I hope it is), you might consider adding a class like .mw-raw-heading for easier targetting.

Change 1003812 merged by jenkins-bot:

[mediawiki/core@master] Inherit text color for `hN` nested inside `.mw-heading`

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

One caveat I thought of with enclosing styled bare <h2>'s with the mw-heading <div>, is that it would prevent margin collapsing with the preceding (or following) element.

Concerning the <h2> that are not enclosed with the <div> on special pages and in the visual editor, is it by choice or is it because it would require processing at other parts of the codebase?

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

[mediawiki/core@master] Move section heading formatting to post-cache transform (take 2)

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

Hello @matmarex
For Tech News - What wording would you suggest as the content, and When should it be included? Thanks!"

Change 1004160 merged by jenkins-bot:

[mediawiki/core@master] Move section heading formatting to post-cache transform (take 2)

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

With only the "new" and the "bare" markups remaining, we can easily target all <h2>'s by simply using h2 selector. If ever we need to select only the bare <h2>'s, we can do :not(.mw-heading) > h2. And to target all headings at the topmost level, we could do .mw-heading2, :not(.mw-heading) > h2.

Correct, but note that if you have a more complicated selector, adding the :not(…) in the right place is tricky. For example, I just updated https://fr.wikipedia.org/wiki/MediaWiki:Common.css, which had selectors like .mw-parser-output h3. To update those using this method, you would need to use .mw-parser-output :not(.mw-heading) > h3, .mw-parser-output > h3, because the heading can be a direct child of .mw-parser-output, or it can be nested inside another element, so you need selectors to handle both cases.

You could use .mw-parser-output h3:not(.mw-heading h3) instead, but this would drop support for some older browsers where the current CSS works fine (https://caniuse.com/css-not-sel-list). This may be acceptable, but I don't want to force anyone to make that decision.

That's why on https://www.mediawiki.org/wiki/Heading_HTML_changes I documented the longer, but simplest method: just style both h2 and .mw-heading2, and then "undo" the styles on .mw-heading2 h2. And that's how I changed the styles too: https://fr.wikipedia.org/w/index.php?title=MediaWiki:Common.css&diff=prev&oldid=212874402 (but feel free to do it differently if you prefer)

While writing this, I'm getting a second thought. Actually, I think we should enclose the bare <h2>'s with a <div class="mw-heading mw-heading2">, and keep the provided attributes on the generated <h2>. It would unify the markup and let simplify the selectors. I can't think of cases it would harm, but maybe you have some? And if this enclosing is not possible (though I hope it is), you might consider adding a class like .mw-raw-heading for easier targetting.

One caveat I thought of with enclosing styled bare <h2>'s with the mw-heading <div>, is that it would prevent margin collapsing with the preceding (or following) element.

If I understand your idea correctly, I'm afraid it wouldn't work because of the default styles MediaWiki applies to .mw-heading h2 etc. – most importantly, it sets them to display: inline, which would change how inline styles like border, background etc. affect them.

Concerning the <h2> that are not enclosed with the <div> on special pages and in the visual editor, is it by choice or is it because it would require processing at other parts of the codebase?

It's a bit of both. It's by choice because we don't want to complicate the markup where it's not needed. But it also helps us avoid updating other parts of the codebase, which would add even more work and more delay to this task.

Hello @matmarex
For Tech News - What wording would you suggest as the content, and When should it be included? Thanks!"

Not sure yet, but I'll take care of it. I'll talk to folks from the Content Transform team before adding an entry, as they might have an opinion on this.

Hello @matmarex
For Tech News - What wording would you suggest as the content, and When should it be included? Thanks!"

Not sure yet, but I'll take care of it. I'll talk to folks from the Content Transform team before adding an entry, as they might have an opinion on this.

Thanks!

Noted in https://meta.wikimedia.org/wiki/Tech/News/2024/10. Current draft:

The HTML markup of headings and section edit links will be changed later this year to improve accessibility. See Heading HTML changes for details. The new markup will be the same as in the new Parsoid wikitext parser. You can test your gadget or stylesheet with the new markup if you add ?useparsoid=1 to your URL (more info) or turn on Parsoid read views in your user options (more info).

DT's subscribe button is now inside span.mw-headline which is inside h2. Doesn't seem right and in line with this task.

image.png (526×1 px, 103 KB)

DT's subscribe button is now inside span.mw-headline which is inside h2. Doesn't seem right and in line with this task.

image.png (526×1 px, 103 KB)

In your screenshot, DT styles seem to be disabled. Two cases:

  • They are disabled in your preferences (Special:Preferences#Editing → Discussion pages → “Show discussion activity”).
  • You’re not in Talk namespace, so “discussion activity” is not shown to avoid some false display at content pages.

@Pols12 Yeah, I know. I have only "Enable topic subscriptions" enabled. My point is – whatever the set of options is, it's not correct for the [ subscribe ] button to appear where it appears. It also didn't appear inside .mw-headline before (I came here after I got a couple of bug reports for Convenient-Discussions), so it's a regression.

When DT is completely disabled, there are placeholders instead of DT buttons. And you can see that the [ subscribe ] button is placed in span.mw-headline while the newer "Subscribe" button (with the bell icon) is placed correctly:

image.png (622×1 px, 126 KB)

@matmarex can you maybe comment on the cause of the above?

Yeah, definitely seems like the subscribe button placement in the headers is different depending on whether visual enhancements are enabled. ext-discussiontools-init-section-subscribeButton winds up outside the h2, whereas ext-discussiontools-init-section-subscribe winds up inside it.

Sorry, I was busy recently and haven't looked at this task.

DT's subscribe button is now inside span.mw-headline which is inside h2. Doesn't seem right and in line with this task.

image.png (526×1 px, 103 KB)

Looks like it's an accidental side-effect of the changes in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1004160. Before that, the button was inside the h2, but outside (before) the span.mw-headline. I didn't notice that when working on the changes.

I'm sorry that this caused work for you. It seems that nothing else was affected and no one else seems to have noticed, so I'm going to leave it like that, until we're ready to change the entire markup.

I'm sorry that this caused work for you. It seems that nothing else was affected and no one else seems to have noticed, so I'm going to leave it like that, until we're ready to change the entire markup.

That’s not true: I had to make a comment here after this being a problem for different people for three times. Autogenerated summaries got affected even if you don’t use CD, and some user scripts and gadgets also were broken by this (see https://ru.wikipedia.org/wiki/Обсуждение_Википедии:Гаджеты/Гаджет_проекта_«Добротные_статьи»#Гаджет_добавляет_слово_"подписаться"_в_список_ДС.).

@stjn I didn't know about that. Well, again, it's my bad.

On second thought, I think we could move them outside of the h2 (where the "Subscribe" buttons are placed when visual enhancements are enabled). This will be different than before, but hopefully it will be better. And (I think, after working on other pieces of this for the last couple of hours) it could make other things easier. So let's try that.

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

[mediawiki/extensions/DiscussionTools@master] Move [subscribe] links outside of `<h2>` tags

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

Sounds like the patch will fix the problem with a gadget on English Wiktionary. MediaWiki:Gadget-aWa.js moves the sections of concluded discussions to talk pages of entries. It currently displays [subscribe] at the beginning of the section name, because it grabs the textContent of mw-headline elements.

The patch would also fix the bug where it grabbed the page title from the [subscribe] link rather than the link in the actual section title (because the subscribe link is the first link in mw-headline), but I came up with a workaround for that already, which I can revert when the patch is live.

Yes, it will. (Although you'll need to update it again when the other changes in this task land – see https://www.mediawiki.org/wiki/Heading_HTML_changes.)

Change 1009793 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Move [subscribe] links outside of `<h2>` tags

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

Change 859139 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Compatibility with new heading HTML

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

Change 857772 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Compatibility with new heading HTML (editor)

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