Closed Bug 1226877 Opened 9 years ago Closed 8 years ago

[a11y] Invisible elements in devtools shouldn't be tabbable (aka accessibility is broken)

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox45 affected, firefox50 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 50
Tracking Flags:
Tracking Status
firefox45 --- affected
firefox50 --- verified

People

(Reporter: arni2033, Assigned: yzen)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: regression)

Attachments

(2 files)

Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools.
58 bytes, text/x-review-board-request
bgrins
: review+
Details
Bug 1226877 - make toolbox panels inaccessible with keyboard when they are hidden or collapsed.
58 bytes, text/x-review-board-request
bgrins
: review+
Details
>>> My Info: Win7_64, Nightly 45, 32bit, ID 20151119030404 STR: 1. Open this "data:" url (only to log something reliably) > data:text/html,<script> for (i=0;i<99;i++) console.log(i); </script> 2. Open developer tools -> Console, then switch to Inspector tab 3. Open "Rules" tab in Inspector sidebar, then close sidebar 4. Click <body> node in markup-view, make sure it's focused 5. Press Tab key twice 6. Press Tab key 20 times Result: After Step 5 it's not clear what element is focused. (But actually it's an element in Inspector Sidebar which is collapsed) After Step 6 a link in console is focused: you can tell it by url tooltip in the bottom left corner of content area Expectations: After Step 5 selected <tab> in TabsToolbar should become focused
Has STR: --- → yes
See Also: → 1248297
Blocks: devtoolsa11y
Thanks for filing this, I noticed that as well but have not opened a bug for it. This is actually a regression as Tabbing was not so crazy a couple of weeks ago.
Keywords: regression
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
OK this patch does not actually fix this particular issue but does other ones, will add a fix to this review request shortly.
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. https://reviewboard.mozilla.org/r/61302/#review58174 The focus on the web console input field is not being set when the split console opens with this patch applied (it does without it) ::: devtools/client/framework/sidebar.js:458 (Diff revision 1) > tab.allTabsMenuItem.setAttribute("checked", true); > } > }, > > /** > + * Set visibility for sidebar panels. The sidebar panels are actively being re-done to use HTML in Bug 1259819, so I'd prefer not to land this in between. Maybe leave a comment in that bug, or file a follow up to address this is needed on top of that work ::: devtools/client/framework/test/browser_toolbox_sidebar.js:113 (Diff revision 1) > let label = 1; > for (let tab of tabs) { > is(tab.getAttribute("label"), label++, "Tab has the right title"); > } > > + testVisibility(panel, 0); Same ::: devtools/client/inspector/inspector.css:37 (Diff revision 1) > * Override `-moz-user-focus:ignore;` from toolkit/content/minimal-xul.css > */ > -moz-user-focus: normal; > } > + > +/** Same
Attachment #8766373 - Flags: review?(bgrinstead) → review-
Another bug with this applied: open the split console (escape), close the split console (escape), try to open again (escape) and it doesn't open
(In reply to Brian Grinstead [:bgrins] from comment #6) > Comment on attachment 8766373 [details] > Bug 1226877 - fixing keyboard navigation around hidden devtools > panels/controls. > > https://reviewboard.mozilla.org/r/61302/#review58174 > > The focus on the web console input field is not being set when the split > console opens with this patch applied (it does without it) > > ::: devtools/client/framework/sidebar.js:458 > (Diff revision 1) > > tab.allTabsMenuItem.setAttribute("checked", true); > > } > > }, > > > > /** > > + * Set visibility for sidebar panels. > > The sidebar panels are actively being re-done to use HTML in Bug 1259819, so > I'd prefer not to land this in between. Maybe leave a comment in that bug, > or file a follow up to address this is needed on top of that work > > ::: devtools/client/framework/test/browser_toolbox_sidebar.js:113 > (Diff revision 1) > > let label = 1; > > for (let tab of tabs) { > > is(tab.getAttribute("label"), label++, "Tab has the right title"); > > } > > > > + testVisibility(panel, 0); > > Same > > ::: devtools/client/inspector/inspector.css:37 > (Diff revision 1) > > * Override `-moz-user-focus:ignore;` from toolkit/content/minimal-xul.css > > */ > > -moz-user-focus: normal; > > } > > + > > +/** > > Same Thanks yes, sidebar tabs are working as expected with changes in bug 1259819
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61302/diff/1-2/
Attachment #8766373 - Flags: review- → review?(bgrinstead)
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. https://reviewboard.mozilla.org/r/61302/#review60372 Works for me locally, but seeing bustage on the try push ::: devtools/client/framework/toolbox.js:1473 (Diff revision 2) > this._refreshConsoleDisplay(); > this.emit("split-console"); > > if (this._lastFocusedElement) { > this._lastFocusedElement.focus(); > + } else if (this.doc.activeElement) { Should we be doing a check here to see if this.doc.activeElement is the webconsole frame? ::: devtools/client/framework/toolbox.js:1476 (Diff revision 2) > if (this._lastFocusedElement) { > this._lastFocusedElement.focus(); > + } else if (this.doc.activeElement) { > + // Do not let the focus remain on a hidden frame, move it to the previous > + // active frame in the deck. > + Services.focus.moveFocus(this.win, this.doc.activeElement, We are actively working on removing chrome priv from our frontend, so I'd like to have a discussion about how we can replace some of the functionality in Services.focus for a normal web page. For instance, in this case is there a way that we could do this without Services? (we don't necessarily need to do it in this bug)
Attachment #8766373 - Flags: review?(bgrinstead) → review-
Attachment #8766373 - Flags: review-
(In reply to Yura Zenevich [:yzen] from comment #10) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=8813659a398c I think here we can simply move focus back onto currently visible iframe in the deck. Ill update the patch.
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61302/diff/2-3/
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61302/diff/3-4/
Attachment #8766373 - Flags: review?(bgrinstead)
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. https://reviewboard.mozilla.org/r/61302/#review62080 ::: devtools/client/netmonitor/netmonitor.css:11 (Diff revision 4) > #toolbar-labels { > overflow: hidden; > } > > +/** > + * Use visibility:; hidden instead of display: none to still receieve events Nit: extra semicolon here ::: devtools/client/netmonitor/netmonitor.css:14 (Diff revision 4) > > +/** > + * Use visibility:; hidden instead of display: none to still receieve events > + * from a hidden pane. > + */ > +#details-pane.pane-collapsed { What is the utility of getting events from the hidden details pane? Do we actually need to be able to navigate inside it when it's hidden? r=me on the toolbox.css changes, but I want to understand this part more before agreeing to it. If you want to land the toolbox parts, feel free to break the netmonitor stuff out into a follow-up bug (up to you).
Attachment #8766373 - Flags: review?(bgrinstead) → review+
As per conversation , will split into 2 patches with more explanation in comments/commit messages.
--- devtools/client/themes/toolbox.css | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) Review commit: https://reviewboard.mozilla.org/r/66096/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/66096/
Attachment #8766373 - Attachment description: Bug 1226877 - fixing keyboard navigation around hidden devtools panels/controls. → Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools.
Attachment #8773399 - Flags: review?(bgrinstead)
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61302/diff/4-5/
https://reviewboard.mozilla.org/r/61302/#review63000 r+, looks like there's some extra junk at the bottom of the commit message that might need cleanup
https://reviewboard.mozilla.org/r/66096/#review63006 Same comment about the end of the commit message ::: devtools/client/themes/toolbox.css:379 (Diff revision 1) > + * inaccessible by keyboard. This is not the case by default because the are > + * predominantly hidden using visibility: collapse; style or collapsed > + * attribute. > + */ > +.toolbox-panel *, > +#toolbox-deck[collapsed] *, The only time the toolbox-deck is collapsed is when the console is selected (so none of the tools in the deck will be selected and won't have their -moz-user-focus reset by the rule below). As such, I don't think this rule is needed. Make sense? Or is there a case I'm not thinking of
Comment on attachment 8773399 [details] Bug 1226877 - make toolbox panels inaccessible with keyboard when they are hidden or collapsed. ni? for Comment 22
Flags: needinfo?(yzenevich)
Attachment #8773399 - Flags: review?(bgrinstead)
(In reply to Brian Grinstead [:bgrins] from comment #23) > Comment on attachment 8773399 [details] > Bug 1226877 - make toolbox panels and decks inaccessible with keyboard when > they are hidden or collapsed. > > ni? for Comment 22 Yep, tested, you are right!
Flags: needinfo?(yzenevich)
Comment on attachment 8766373 [details] Bug 1226877 - make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61302/diff/5-6/
Attachment #8773399 - Attachment description: Bug 1226877 - make toolbox panels and decks inaccessible with keyboard when they are hidden or collapsed. → Bug 1226877 - make toolbox panels inaccessible with keyboard when they are hidden or collapsed.
Attachment #8773399 - Flags: review?(bgrinstead)
Comment on attachment 8773399 [details] Bug 1226877 - make toolbox panels inaccessible with keyboard when they are hidden or collapsed. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66096/diff/1-2/
Attachment #8773399 - Flags: review?(bgrinstead) → review+
Comment on attachment 8773399 [details] Bug 1226877 - make toolbox panels inaccessible with keyboard when they are hidden or collapsed. https://reviewboard.mozilla.org/r/66096/#review63222
Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b45c5745abc0 make collapsed details pane hidden using visibility: hidden to be inaccessible by both keyboard and a11y tools. r=bgrins https://hg.mozilla.org/integration/mozilla-inbound/rev/a5ee1d48d2f5 make toolbox panels inaccessible with keyboard when they are hidden or collapsed. r=bgrins
https://hg.mozilla.org/mozilla-central/rev/b45c5745abc0 https://hg.mozilla.org/mozilla-central/rev/a5ee1d48d2f5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
I have reproduced this bug with Nightly 45.0a1 (2015-11-21) on Windows 7, 64 Bit! This bug's fix is verified with latest Aurora Build ID 20160803004014 User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0 [bugday-20160803]
Reproduced this bug in firefox nightly 45.0a1 (2015-11-21) as comment 0 with ubuntu 16.04 (64 bit) Verified this bug as fixed with latest firefox aurora 50.0a2 (Build ID: 20160804004004) Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
QA Whiteboard: [bugday-20160803]
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Depends on: 1318048
Depends on: 1327789
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: