Page MenuHomePhabricator

Implement accessibility jump links with pure CSS
Closed, ResolvedPublic

Description

We currently load a module jquery.mw-jump on all pages in MediaWiki core (via the mediawiki.page.ready module).

As part of T192623, I was going to merge the mw-jump code into mediawiki.page.ready, but then I realised, this is quite trivial logic and would actually be simpler, faster, and better for accessibility if it were done in pure CSS. See the Gerrit patches linked below for specifics.

Currently

MediaWiki core defines the following three messages:

i18n/en.json
	"jumpto": "Jump to:",
	"jumptonavigation": "navigation",
	"jumptosearch": "search",

And the following JavaScript code:

mw-jump.js
$( '.mw-jump' ).on( 'focus blur', 'a', function ( e ) {
	if ( e.type === 'blur' || e.type === 'focusout' ) {
		$( this ).closest( '.mw-jump' ).css( { height: 0 } );
	} else {
		$( this ).closest( '.mw-jump' ).css( { height: 'auto' } );
	}
} );

Neither the messages nor the JavaScript is actually used by MediaWiki core (the messages aren't output, and the script isn't triggered, by something from the OutputPage, Skin or SkinTemplate classes). Instead, the various skins that used to be part of core (Vector, MonoBook, ..), use the messages in their own repos, and also craft (similar) HTML that is compatible with this script, depending on the fact that core will queue the script as part of mediawiki.page.ready.

This is essentially left-over technical debt from the splitting of the skins.

Proposed

The Vector and MonoBook skins will use their own messages, and use their own HTML/CSS for these accessibility links.

Once that is done, we can deprecate the messages and modules; to be removed in the next major release.

Event Timeline

Change 434352 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/skins/Vector@master] Re-implement and improve mw-jump links with pure CSS

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

Change 434832 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/skins/MonoBook@master] Re-implement and improve mw-jump links with pure CSS

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

Change 434352 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Re-implement and improve mw-jump links with pure CSS

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

Change 434832 merged by jenkins-bot:
[mediawiki/skins/MonoBook@master] Re-implement and improve mw-jump links with pure CSS

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

Change 434905 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Remove mentions of jquery.mw-jump in code comments

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

Change 434905 merged by jenkins-bot:
[mediawiki/core@master] Remove mentions of jquery.mw-jump in code comments

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

Krinkle triaged this task as High priority.Jun 4 2018, 8:02 PM

Hmm, I think we need to remove:

	#jump-to-nav {
		/* Negate #contentSub's margin and replicate it so that the jump to links don't affect the spacing */
		margin-top: -1.4em;
		margin-bottom: 1.4em;
	}

https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.skinning/interface.css#L66

because it seems we are losing margin on the top of the page. See also:
https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)?debug=true#Beginning_of_text_too_close_to_the_top_of_the_page
and T178626: #jump-to-nav margin elimination caused the indentation between the subtitle and the article content to disappear

@TheDJ Yep, you're right. In an earlier version of the patch, I avoided this issue by not leaving the (now unused) #jump-to-nav behind, but we had to put it back for compatibility with older gadgets using it as a hooking point to insert otherwise unrelated interface elements.

Removing the styles entirely would break skins using the old jumper format. As of writing, I intentionally left the jquery.mw-jump and #jump-to-nav styles untouched so that other skins are not affected or required to change.

However, given that this is incompatible we'll have to at least a minimal change.

I propose the following:

  • Change the old styles in core (not used by Vector/MonoBook) to declare rules based on class instead of ID (e.g. .jump-to-nav). This is effectively the same as removing them and will solve the spacing issue we see on Wikipedia today.
  • Then, we update any other skins we find in Git to add the class attribute to their <div id="jump-to-nav"> element, with release notes indicating this is a stop-gap measure and that from 1.32 or 1.33 onwards, skins should have styles for these in their own stylesheets instead of depending on core.

Alternatively, given it is just three lines of CSS, it might make sense to remove it straight away, thus still requiring that skins be updated, but instead of updating their HTML slightly, we'd be asking them to add the same styles as before, and we'd remove them from core.

Change 438007 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.skinning: Remove styles for #jump-to-nav from core

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

@Krinkle As i was reading your commit i came up with #jump-to-nav:not(:empty)
I was also thinking: If we leave dom in place that is deprecated, for the sake of scripts... What if we add a class="mw-deprecated" or something to that ?

@TheDJ Such .mw-deprecated class sounds like a recipe to remain in code for the next 10+ years. I don't see this as a useful or clear indicator for (volunteer) devs.

@Volker_E the intent was using it for ourselves, not for script authors. Easier to find back than a freeform todo comment in some random repo. Also scannable with automation tools within the live website, which seems like it could be useful

Change 438007 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.skinning: Remove styles for #jump-to-nav from core

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