Last modified: 2014-02-06 14:59:37 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T60255, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 58255 - mediawiki.util should not use hardcoded skin-specific selectors to find accesskey elements
mediawiki.util should not use hardcoded skin-specific selectors to find acces...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
unspecified
All All
: Low normal (vote)
: 1.23.0 release
Assigned To: Bartosz Dziewoński
: accessibility
Depends on:
Blocks: 27292
  Show dependency treegraph
 
Reported: 2013-12-10 05:41 UTC by Isarra
Modified: 2014-02-06 14:59 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Isarra 2013-12-10 05:41:00 UTC
Access keys don't work in custom skins without a core hack, apparently. Kind of strange.
Comment 1 Derk-Jan Hartman 2013-12-10 09:28:29 UTC
This is very little information to go on....
Comment 2 Jack Phoenix 2013-12-10 10:28:42 UTC
(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.
Comment 3 Derk-Jan Hartman 2013-12-10 11:28:32 UTC
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 ?
Comment 4 Andre Klapper 2013-12-10 13:25:19 UTC
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.
Comment 5 MZMcBride 2013-12-10 15:56:45 UTC
(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"?
Comment 6 Isarra 2013-12-10 18:00:27 UTC
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.
Comment 7 Bartosz Dziewoński 2013-12-10 18:07:30 UTC
(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.
Comment 8 Andre Klapper 2013-12-10 18:14:39 UTC
(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...
Comment 9 Isarra 2013-12-10 19:15:01 UTC
(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.
Comment 10 Isarra 2013-12-10 19:16:54 UTC
Also I didn't actually know myself what specific skins it applied to at the time, but never mind that.
Comment 11 Derk-Jan Hartman 2013-12-11 07:56:04 UTC
@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.
Comment 12 Isarra 2013-12-11 08:22:30 UTC
(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?
Comment 13 Gerrit Notification Bot 2014-01-30 15:37:11 UTC
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
Comment 14 Gerrit Notification Bot 2014-02-06 13:44:14 UTC
Change 110377 merged by jenkins-bot:
mediawiki.util: Don't hardcode selectors in updateTooltipAccessKeys if possible

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links