Page MenuHomePhabricator

Article/talk access keys not working while elements are hidden with responsive MonoBook
Closed, ResolvedPublic

Description

From enwp's VPT:

@Isarra: The article/talk access keys don't work; the "edit this page" link (and it turns out all the others!) work fine. Graham87 16:50, 1 June 2018 (UTC)

Event Timeline

Legoktm created this task.Jun 1 2018, 7:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 1 2018, 7:28 PM

This is unlikely to be caused by responsive monobook, as no changes were made to any of the access keys. The entire cactions block is hidden in mobile when js is enabled, but this includes edit etc, so if edit etc still work, that's not it either.

Graham87 renamed this task from Article/talk access keys not working after responsive MonoBook to Article/talk access keys not working while elements are hidden with responsive MonoBook.Jun 2 2018, 9:52 AM

I've changed the title to clarify exactly when the access keys aren't working, per the comments I just made about this issue at the village pump discussion. On the prototype, I can reproduce this issue in both JAWS and NVDA, the two most popular Windows screen readers, in both IE and Firefox (using the latest stable versions of all these programs)

stjn added a subscriber: stjn.Jun 2 2018, 9:11 PM

Probably what would fix this is visually hiding specific menus instead of hiding them entirely with display:none;. I would guess that they don’t work exactly by that logic: if they aren’t displayed in browser, they aren’t actionable. However the edit button working is definitely strange and can’t be described by this.

Isarra added a comment.Jun 3 2018, 7:58 AM

Yeah, the clarification says it is the display:none after all. There's actually a mixin in monobook's variables.less already for hidifying things properly that we can use, so the fix is really easy here.

Isarra added a comment.Jun 3 2018, 7:59 AM

Apparently the mix of things working was just... in different circumstances, or such.

Change 437468 had a related patch set uploaded (by Isarra; owner: Isarra):
[mediawiki/skins/MonoBook@master] Hide dropdowns instead of removing for screen readers

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

Change 437468 merged by jenkins-bot:
[mediawiki/skins/MonoBook@master] Hide dropdowns instead of removing for screen readers

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

@Graham87 You can test this change now on the Beta Cluster (https://en.wikipedia.beta.wmflabs.org/). Please let us know if it works well for you, and if it does, I will get it deployed to the production Wikimedia wikis soon as well (otherwise it would have to wait until the scheduled deployments next week).

I would test it but that cluster's been down for an hour now ...

Sorry about that. It was working when I was writing my last comment, and it seems to be working for me again right now.

I can't get the elements to be hidden on the beta labs cluster. However on Wikipedia it seems to have returned again with the same problems.

Isarra added a comment.Jun 8 2018, 7:43 AM

You don't actually benefit any from the responsive mode being any different, do you? Would it make sense for it to effectively remain the same as before the change, to screen readers?

You don't actually benefit any from the responsive mode being any different, do you? Would it make sense for it to effectively remain the same as before the change, to screen readers?

Exactly. However, I don't know precisely how that'd work without causing other side effects, seeing as as far as I know there's no non-invasive way to detect the presence of a screen reader. I do appreciate the new preference to turn it off universally though.

An aria-hidden="true" smacked on any new redundant things should resolve that for screen readers.

As a side note, this produces a very weird experience for non-screenreader keyboard navigation, as the now-invisible links can get focus now when tabbing (try e.g. clicking the search box, then pressing Tab a couple times).
You might want to make the menus appear when they get focused (either with JS, or with :focus-within pseudoclass, I'm not sure which will be easier). But that's a corner-case (these styles are meant for devices with no keyboards, after all) so I'm merging this as-is.

Not sure how to resolve this issue for people on tiny screens, though.

Should probably break this off into a new task for cleaning the fix for this task up, or stuff.

Vvjjkkii renamed this task from Article/talk access keys not working while elements are hidden with responsive MonoBook to ltbaaaaaaa.Jul 1 2018, 1:06 AM
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot renamed this task from ltbaaaaaaa to Article/talk access keys not working while elements are hidden with responsive MonoBook.Jul 2 2018, 1:32 AM
CommunityTechBot raised the priority of this task from High to Needs Triage.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added subscribers: gerritbot, Aklapper.
Evad37 moved this task from Backlog to Responsive MonoBook on the MonoBook board.Aug 21 2018, 7:29 AM
Isarra added a comment.Jan 9 2019, 6:52 PM

Is this still broken?

Nope, the access keys are working fine here now.

Isarra closed this task as Resolved.Jan 10 2019, 12:49 AM
Isarra claimed this task.

Awesome, thanks!