Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
ButtonWidget: Remove code to not let the button get focus after clicking | oojs/ui | master | +7 -24 |
Details
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Resolved | • Prtksxna | T87714 Return focus to the last location when a modal dialog is dismissed | |||
Resolved | • Prtksxna | T76636 Make buttons steal focus after click unless configured otherwise |
Event Timeline
So in other words, make the weird tabindex hack in ButtonElement optional? Why do we even have this hack? (Why do we prevent "focus stealing"?) It was added in 2120e338e6605e076638c2d27a88e72756030d40 and there's no explanation there as to why.
@TrevorParscal, do you by any chance remember what was the rationale for buttons not taking focus when clicked?
@TheDJ, do you see any accessibility-related reason why buttons shouldn't take focus when clicked?
it's probably done, because otherwise it steals the focus from the editable area, this is often not desired in toolbars for textual input (because you loose your cursor position).
For keyboard accessibility, you would therefor use keyboard combinations (as documented in the help) menu. you don't go hopping around with your focus to use a button, while you are trying to write text into an area.
For screen readers, the focus doesn't really matter, the focus (mostly) follows the accessibility cursor automatically.
Yeah, but we don't use ButtonElement in toolbars (e.g. in VE, the "Save page" button is the only actual button, everything else is toolbar stuff).
I noticed that on Firefox and Chrome on MacOS the buttons don't get focus even after removing the hack from ButtonWidget. This is only the case with OOui buttons, normal buttons and links get focus, but not the visible focus state.
Explicitly giving them focus onMouseDown works
I was looking at this again and apparently the explicit focus was required because ButtonElement sets [[ https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.mixin.ButtonElement-static-property-cancelButtonMouseDownEvents | cancelButtonMouseDownEvents ]] to true. @matmarex, Would changing this for ButtonElement cause issues with the Toolbar?
Change 281638 had a related patch set uploaded (by Prtksxna):
ButtonWidget: Remove code to not let the button get focus after clicking
With Toolbar? Probably not. I have no idea if there's something that would be negatively affected. I think we should try and see.
Change 281638 merged by jenkins-bot:
ButtonWidget: Remove code to not let the button get focus after clicking