Page MenuHomePhabricator

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

Assigned To
Authored By
bzimport
Oct 3 2007, 9:51 PM
Referenced Files
F54018214: RPReplay_Final1716260739.webm
May 21 2024, 3:48 AM
F54018204: RPReplay_Final1716260654.webm
May 21 2024, 3:48 AM
F54018391: 2024-05-21 04-57-31.webm
May 21 2024, 3:48 AM
F54018378: 2024-05-21 04-56-36.webm
May 21 2024, 3:48 AM
F42321156: image.png
Mar 2 2024, 6:30 AM
F42310324: image.png
Mar 2 2024, 12:48 AM
F41548445: image.png
Nov 30 2023, 11:23 AM
F54414: HolyGrail.htm
Mar 3 2015, 11:55 PM
Tokens
"Love" token, awarded by stjn."Love" token, awarded by Ladsgroup."Mountain of Wealth" token, awarded by Volker_E."Like" token, awarded by He7d3r.

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 through the headlines. At every single headline they hear "[edit]" first[ann.: changed to behind the section title, still in each of them at the end] 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.

Details

Reference
bz11555
SubjectRepoBranchLines +/-
mediawiki/skins/Vectormaster+78 -6
mediawiki/skins/Vectormaster+33 -18
mediawiki/extensions/PageImagesmaster+4 -2
mediawiki/extensions/LabeledSectionTransclusionmaster+16 -6
mediawiki/extensions/Scribuntomaster+9 -4
mediawiki/skins/MonoBookmaster+1 -0
mediawiki/skins/Timelessmaster+1 -0
mediawiki/skins/Nostalgiamaster+1 -0
mediawiki/skins/Examplemaster+1 -0
mediawiki/coremaster+3 K -536
mediawiki/skins/MinervaNeuemaster+4 -2
mediawiki/extensions/MobileFrontendmaster+3 -3
mediawiki/extensions/VisualEditormaster+6 -3
mediawiki/extensions/DiscussionToolsmaster+336 -338
mediawiki/coremaster+38 -39
mediawiki/coremaster+1 -0
mediawiki/coremaster+37 -39
mediawiki/coremaster+55 -2
mediawiki/extensions/DiscussionToolsmaster+14 -9
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
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

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

Change #860582 abandoned by Bartosz Dziewoński:

[mediawiki/skins/MinervaNeue@master] Compatibility with new heading HTML

Reason:

No longer needed

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

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

[mediawiki/skins/Vector@master] Compatibility with new heading HTML (table of contents)

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

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

[mediawiki/skins/CologneBlue@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/Example@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/MinervaNeue@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/Modern@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/MonoBook@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/Nostalgia@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/Timeless@master] Indicate support for new heading HTML

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

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

[mediawiki/skins/Vector@master] Indicate support for new heading HTML

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

Just to make sure web team schedule sufficient time for this work, when are we expecting to merge https://gerrit.wikimedia.org/r/c/mediawiki/core/+/842859 ?

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

[mediawiki/core@master] Enable new heading HTML by default

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

Change #842859 merged by jenkins-bot:

[mediawiki/core@master] Move section edit links outside headings (new heading HTML)

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

Just to make sure web team schedule sufficient time for this work, when are we expecting to merge https://gerrit.wikimedia.org/r/c/mediawiki/core/+/842859 ?

The patch has been merged, but it's active until we deploy a config change. That can be done when web team is ready.

Change #1022355 merged by jenkins-bot:

[mediawiki/skins/Example@master] Indicate support for new heading HTML

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

Change #1022359 merged by jenkins-bot:

[mediawiki/skins/Nostalgia@master] Indicate support for new heading HTML

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

Change #1022360 merged by jenkins-bot:

[mediawiki/skins/Timeless@master] Indicate support for new heading HTML

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

Change #1022358 merged by jenkins-bot:

[mediawiki/skins/MonoBook@master] Indicate support for new heading HTML

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

Change #860555 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Update parser tests for new heading HTML

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

For anyone following along, the changes for the skins mentioned above are now being released to Wikimedia wikis. Other skins will follow later. The announcements are handled in T365078.

I updated the help page at https://www.mediawiki.org/wiki/Heading_HTML_changes to reflect how the patches evolved, and added the currently expected timeline. It's happening.


I have also realized that no one has ever recorded what the current experience of navigating by headings feels like for screen reader users, and how the changes improve it, so I did that. Make sure to unmute the audio, and consider that I'm a beginner screen reader user myself :)

Using Windows Narrator – I am using touch gestures (I'm told this isn't the software nor the method favored by the more proficient users, but it was the most accessible for me):

Using iOS VoiceOver – I am using the "rotor":

Change #860577 merged by jenkins-bot:

[mediawiki/extensions/LabeledSectionTransclusion@master] Update parser tests for new heading HTML

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

Change #860553 merged by jenkins-bot:

[mediawiki/extensions/PageImages@master] Update parser tests for new heading HTML

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

FYI, I have just added support for the new markup in this gadget on frwiki: https://fr.wikipedia.org/w/index.php?title=MediaWiki:Gadget-AncreTitres.js&diff=215319381&oldid=213586059

In this gadget, for each section heading, we need its topmost container (we append a link to it), and the id attribute.

There should be no overlap across the selections of old and new markups, as it would bring various errors (for instance, duplicate processing). I figured out that decoupling the processing to a dedicated function led to a simpler code.
(I also took the opportunity to make the code robuster, in case of a malicious user inputting "mw-" class in a page's wikicode, so that there is no id attribute, then an undefined id property at JavaScript runtime)

I'm not sure if this is the correct ticket - currently we are having some issues with javascript gadgets like https://commons.wikimedia.org/wiki/Help:Gadget-DelReqHandler on commons.wikimedia in the monobook skin only which maybe related to this change. Btw, gadgets in vector skin works. There is also a discussion on commons related to this topic: https://commons.wikimedia.org/wiki/Commons:Administrators%27_noticeboard#Problem_with_DelReqHandler

Maybe someone with more detailed knowledge about the background and changes could have look on this.

For the start, please check and follow https://www.mediawiki.org/wiki/Help:Locating_broken_scripts if there are any errors. Thanks!

@Wdwd I replied there, I'm trying to fix the gadget. @Izno @Od1n Thanks for making these fixes, I added them as examples for others to https://www.mediawiki.org/wiki/Heading_HTML_changes (and feel free to edit that page with any of your own advice).

Would it be possible to reintroduce mw-headline as class for the headers h#? This may help in updating some tools easier. Using the example from Heading HTML changes in MediaWiki I think of

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

Update: At time of writing about 3000 uses in scripts and user styles:
Global Search for mw-headline.

This has broken around 5 gadgets on enwiki so far, and 3 user scripts that I use. It's going to be a lot of work to fix these. Several are unmaintained.

Can https://www.mediawiki.org/wiki/Heading_HTML_changes please be updated with the exact timeline of when what skins are being affected? For example vector 2010 deployed to enwiki today but the Timeline section doesn't say this yet. I would be interested in knowing the exact dates for the upcoming vector-2022 and minerva deployments.

The timeline is flexible. Thankfully Bartosz built this feature with flexible roll out in mind. We've been intentionally ramping up the deployment by number of users using easy skins. The current plan was Minerva next week (T365736), and Vector 2022 in the two weeks that followed but we've been adjusting the timeline as needed as we don't want to overwhelm gadget developers like you. The plan is to get all these changes done in the 1.43 release.

Let me know how long you would need? I'm happy to delay this roll out a bit longer before exposing the new HTML to more people and breaking more things!

The fact that not all skins are being migrated at the same time, causes us to have to support both syntaxes at the same time. This is quite annoying.

It'd also have been nice to get a heads up about the syntax change on our Village pump about this change. Fortunately, someone was able to track down what had changed and I have fixed two broken gadgets and broken styles in our Common.css on svwiktionary.

@Skalman, this is by design - you need to support both syntax. Since our projects are highly trafficked we make use of caching, so there will be a transition period where it is possible to get both the old and new markup on page views.

T337286 proposes an API that would have handled this for you, but so far it is lacking details in what gadget developers would find useful - so your expertise would be appreciated there.

I'd be in favor of the current timeline or a quicker timeline. Im leaning toward fixing everything in three weeks to avoid double work and complicated diffs. Except for XFDcloser which is essential so I already fixed it.

I have fixed another gadget: https://fr.wikipedia.org/w/index.php?title=MediaWiki:Gadget-FlecheHaut.js&diff=215986753&oldid=209440727.

It's horrible.

  • In this case, we have to directly grab the desired elements, i.e. grab only them. Contrarily to the "upgraded CSS" here, where the .mw-heading2 h2 ruleset is used to discard some of the applied CSS, as the .mw-heading2, h2 ruleset above grabs too many elements.
  • Even on the day the old markup is fully dropped, selecting the elements will be a pain, because of the support of manually input <hN> tags in wikicode, which produces another markup.

Edit: I had to add another patch, because the #mw-toc-heading element was unexpectedly selected… see https://fr.wikipedia.org/w/index.php?title=MediaWiki:Gadget-FlecheHaut.js&diff=prev&oldid=216009397.

I have (again) noticed another inconsistency with "manually input <hN> tags" in wikicode:

  • If the tag doesn't have attributes, e.g. <h2>Foobar</h2>, it gets enclosed in a .mw-heading container.
  • If the tag has attributes, e.g. <h2 style="...">Foobar</h2>, it doesn't get enclosed in a container…

What I would suggest:

  • Never enclose these in a container, for consistency. (The opposite, always enclose in a container, would cause various CSS issues with the user-specified inline CSS.)
  • Add a .mw-raw-heading class to these elements.

(That's something I had already thought of, see this above comment and the following ones, in particular this reply by matmarex.)

Therefore, to get the topmost element of all headings, we could then do .mw-heading, .mw-raw-heading. Other use cases might still not be as simple yet (get all <hN> elements (taking care of avoiding the #mw-toc-heading element), get id attributes…).

I feel that there will still be room for improvement, but that adding this class would already be a great step forward.

As an example, once the old markup is fully removed and the .mw-raw-heading class is implemented, the gadget update here (mentioned in this message, note it is collapsed) could be improved to the following:

// regular MediaWiki "== Foobar ==" headings (.mw-heading > hN)
$( '.mw-heading' ).each( function ( _, mwHeading ) {
    var heading = mwHeading.querySelector( 'h1, h2, h3, h4, h5, h6' );
    if ( heading && heading.id ) {
        processHeading( mwHeading, heading.id );
    }
} );

// manually input <hN> tags in wikicode (hN.mw-raw-heading)
$( '.mw-raw-heading' ).each( function ( _, rawHeading ) {
    if ( rawHeading.id ) {
        processHeading( rawHeading, rawHeading.id );
    }
} );

Currently, the gadget doesn't support the "manually input <hN> tags", so the 2nd part of the above code is a brand new addition.

In practice, in scripts and styles, forgetting to handle the .mw-raw-heading elements would cause no harm, the only issue is that some headings would not be processed.


Remaining considerations:

  • Special pages, that output plain <hN> tags (without adding the .mw-raw-heading class).
  • h2#mw-toc-heading that may be unexpectedly selected, if directly grabbing the h2, h3... elements.

@Od1n the ‘inconsistency’ you are talking about was implemented in T353489: DiscussionTools's <div class="mw-heading"> wrapper (and future core wrapper) makes it difficult to style headings in wikitext. The reason why attributes are needed to not add .mw-heading wrapper is because of the difficulty to distinguish <h2> from == in the parser, see T353489#9416500. Given that the ‘fix’ is just to add any argument to the heading code, I don’t think this needs to be fixed.

Change #860583 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Compatibility with new heading HTML (table of contents)

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

matmarex closed this task as Resolved.EditedAug 3 2024, 6:57 PM

This change is complete, and it has been deployed to all skins on all Wikimedia wikis about two weeks ago. Thank you to everyone who has been updating scripts and styles on-wiki to support it; I hope it wasn't too disruptive.

If you need to update any more on-wiki code, note that it can be done in a simpler way now, since the old markup can't appear in pages on Wikimedia wikis any more. Code that was updated already can be simplified. (Also, if there are still any tools that no longer work, which are being missed and which have no one to update them, message me on-wiki and I'll try to help.)

Skins, extensions and other code that can be used outside of Wikimedia wikis still need to support both kinds of markup for now. I filed tasks T371755 and T371756 so that we don't forget to remove the old markup completely in the future, at which point that code will also be able to be simplified.

I have updated a gadget to remove support of the old markup, and it didn't become simpler actually: https://fr.wikipedia.org/w/index.php?title=MediaWiki:Gadget-FlecheHaut.js&diff=prev&oldid=217355788.

That's entirely because of the the raw headings. 😭🤡

Just a nitpick, I'm unsure if the class I'm suggesting should be named .mw-raw-heading or .mw-rawheading.
Because if we were also adding hN-specific classes (for completeness with the .mw-headingN classes, and for using instead of hN.mw-raw-heading), the set .mw-heading2 / .mw-raw-heading2 sounds wrong to me, I think .mw-heading2 / .mw-rawheading2 would be more adequate.

(edit: I would bend towards .mw-raw-heading, for consistency with .mw-first-heading, #mw-content-text, etc. and we shoudn't need .mw-raw-headingN classes, hN.mw-raw-heading being fine)

Please consider adding that .mw-rawheading class… Really.

It would let change this:

mw.hook( 'wikipage.content' ).add( function ( $content ) {

    var $pageContent = getPageContent( $content );
    if ( !$pageContent ) {
        return;
    }

    // see: [[mw:Heading HTML changes]] and [[phab:T13555]]

    // regular headings
    var $regularHeadings = $pageContent.find( '.mw-heading' );

    // raw headings (<hN> tags in wikicode, also in special pages)
    // (reminder: special pages have structure #mw-content-text > hN)
    var $rawHeadings = $pageContent
        .find( 'h2, h3, h4, h5, h6' )
        .not( ( _, elm ) => elm.parentNode.matches( '.mw-heading' ) );

    // we also have to filter out the TOC heading <h2>...
    $rawHeadings = $rawHeadings.not( '#mw-toc-heading' );

    $regularHeadings.add( $rawHeadings ).append( $lien );
} );

function getPageContent( $content ) {
    /*
     * Sélection plus précise de l'élément avec le contenu de page,
     * pour éviter l'interface de diff, l'interface de modification, le print footer...
     */
    var $parserOutput = $content.find( '.mw-parser-output' );
    if ( $parserOutput.length ) { // élément avec le contenu de page
        return $parserOutput;
    } else if ( $content.hasClass( 'mw-parser-output' ) ) { // [[phab:T349298]]
        return $content;
    } else if ( mw.config.get( 'wgNamespaceNumber' ) === -1 ) { // on utilise #mw-content-text pour les pages spéciales
        return $content;
    } else { // pas de contenu de page (notamment, pages d'historiques)
        return false;
    }
}

To this:

mw.hook( 'wikipage.content' ).add( function ( $content ) {

    // see: [[mw:Heading HTML changes]] and [[phab:T13555]]

    $content.find( '.mw-heading, .mw-raw-heading' ).append( $lien );
} );