Page MenuHomePhabricator

mediawiki.util should not use hardcoded skin-specific selectors to find accesskey elements
Closed, ResolvedPublic

Description

Access keys don't work in custom skins without a core hack, apparently. Kind of strange.


Version: unspecified
Severity: normal

Details

Reference
bz58255

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 2:37 AM
bzimport set Reference to bz58255.
Isarra created this task.Dec 10 2013, 5:41 AM
TheDJ added a comment.Dec 10 2013, 9:28 AM

This is very little information to go on....

(In reply to comment #1)

This is very little information to go on....

I think that the original report is pretty clear and concise.

But anyway, here goes the longer version.

As a skin developer, I want to build a cool skin -- something unlike anything else that's out there. I use MediaWiki's skin-related classes (BaseTemplate et al.) correctly, because I like to do things right. When I've struggled enough with MediaWiki's JS stuff (regarding the AJAX page watching, for example -- while a separate issue, /resources/mediawiki.page/mediawiki.page.watch.ajax.js assumes way too many things specific to core MW), I test my skin, preferrably in various different browsers, and once I've sorted out all the remaining issues, I notice that accesskeys don't work.

Were I a newbie dev, at this point I'd be asking, "how the hell is this possible?! I did everything by the book!". What the book doesn't tell you is that core (JS) modules make stupid assumptions. Namely, I'm talking about the updateTooltipAccessKeys function in mediawiki.util (/resources/mediawiki/mediawiki.util.js). The core list has been hard-coded to '#column-one a, #mw-head a, #mw-panel a, #p-logo a, input, label', which works for Monobook, Vector and some (but definitely not all) other skins.

Let's take a live example: the Monaco skin (http://www.shoutwiki.com/w/index.php?title=Main_Page&useskin=monaco). Page-specific action links (Monobook's #p-cactions) are in ul#page_controls (and ul#page_tabs). Try hovering your mouse over the "history" link, for example. The title is (and will remain as) "Past revisions of this page [h]" -- note the "h" enclosed in brackets; it should be "[alt-h]" or something similar (I'm using Internet Explorer 11 on Windows 7). The underlying PHP code looks like what you'd expect:
<a href="<?php echo htmlspecialchars( $this->data['content_actions']['history']['href'] ); ?>" title="<?php echo Linker::titleAttrib( 'ca-history', 'withaccess' ) ?>" accesskey="<?php echo Linker::accesskey( 'ca-history' ) ?>">

I've had to hack the updateTooltipAccessKeys function in mediawiki.util to take this ID -- and our other custom skins' various IDs and other selectors -- into account (but that change is not yet live on ShoutWiki), but this seems far from ideal.

There's, of course, the possibility that I've just been outright stupid here. If that's the case, then please do advise me. However, as I'm sure everyone around here is aware, the docs suck -- especially regarding skinning.

AJAX page watching and it's somewhat recent changes are a good example, IMO: in the past to support that feature, all you needed was a hidden div that had the ID "mw-js-message". Apparently this div is nowadays obsolete and AJAX page watching support requires the watch link to have the ID "ca-watch" (or ca-unwatch) or some other ID from the small list of hard-coded IDs. Then again, that's hardly that module's *only* issue -- not everyone uses <ul>s and <li>s like Monobook/Vector do.

So your access keys work, you just don't get the right tooltips, because the scripts make (undocumented) assumptions about the presence of certain classes and IDs ?

See https://www.mediawiki.org/wiki/How_to_report_a_bug - steps to reproduce and about your setup (skin, browser and version, as a start) would be great.

(In reply to comment #2)

Were I a newbie dev, at this point I'd be asking, "how the hell is this
possible?! I did everything by the book!". What the book doesn't tell you is
that core (JS) modules make stupid assumptions. Namely, I'm talking about the
updateTooltipAccessKeys function in mediawiki.util
(/resources/mediawiki/mediawiki.util.js). The core list has been hard-coded
to '#column-one a, #mw-head a, #mw-panel a, #p-logo a, input, label', which
works for Monobook, Vector and some (but definitely not all) other skins.

This seems like the bug to me. What about a bug summary of "mediawiki.util.js hardcodes stupid access key selectors"?

Thank you, MZMcBride.

Derk-Jan, please bear in mind that when a bug comes up, the submitter will not necessarily know what's causing a problem either, only that it doesn't work or what have you - the same little information you complain about may be all we may have as well, but a bug is a bug. Expecting folks to always go into detail when they have no idea where it's coming from themselves, nor any idea how to debug it, will just encourage people to not bother to report bugs, when part of the entire point of reporting bugs is so that someone who knows more can maybe look into the matter.

Andre, as Jack and I said, it applies to non-standard skins (and has apparently done so for years). It's browser-inspecific, though it may look slightly different in different ones.

(In reply to comment #6)

Derk-Jan, please bear in mind that when a bug comes up, the submitter will
not
necessarily know what's causing a problem either, only that it doesn't work
or
what have you - the same little information you complain about may be all we
may have as well, but a bug is a bug. Expecting folks to always go into
detail
when they have no idea where it's coming from themselves, nor any idea how to
debug it, will just encourage people to not bother to report bugs, when part
of
the entire point of reporting bugs is so that someone who knows more can
maybe
look into the matter.

At this point we ask them to provide a screenshot, and the second anyone would see the "bare" accesskeys the reason would become obvious.

(In reply to comment #6)

please bear in mind that when a bug comes up, the submitter will
not necessarily know what's causing a problem either

The request wasn't to analyze the problem already, but to create a good bug report with sufficient steps to reproduce and browser information...

(In reply to comment #8)

(In reply to comment #6)

please bear in mind that when a bug comes up, the submitter will
not necessarily know what's causing a problem either

The request wasn't to analyze the problem already, but to create a good bug
report with sufficient steps to reproduce and browser information...

I'm sorry that I forgot to specifically mention a particular skin or that it is a cross-browser issue, I thought it was redundant with the fact that I said 'custom skins' and didn't specify a specific browser.

Also I didn't actually know myself what specific skins it applied to at the time, but never mind that.

TheDJ added a comment.Dec 11 2013, 7:56 AM

@Isarra, sorry if you think my response was blunt, but the key to any bug report is context.

The only useful information I could gather from your report was: 'custom not working'. But i don't know what custom looks like (it's not in our git repo) and 'not working' can be anything from user error to a crashed computer.

If people start writing their own php scripts, i expect them to be able to write a proper bug report. Handholding is reserved for the less advanced users.

(In reply to comment #11)

@Isarra, sorry if you think my response was blunt, but the key to any bug
report is context.
The only useful information I could gather from your report was: 'custom not
working'. But i don't know what custom looks like (it's not in our git repo)
and 'not working' can be anything from user error to a crashed computer.
If people start writing their own php scripts, i expect them to be able to
write a proper bug report. Handholding is reserved for the less advanced
users.

Does Jack Phoenix's explanation not go into sufficient detail?

Change 110377 had a related patch set uploaded by Bartosz Dziewoński:
mediawiki.util: Don't hardcode selectors in updateTooltipAccessKeys if possible

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

Change 110377 merged by jenkins-bot:
mediawiki.util: Don't hardcode selectors in updateTooltipAccessKeys if possible

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