Improve support for combo boxes in all browsers#8687
Conversation
…ject is a descendant of the document by searching the ancestry for the document's runtimeID with a treeWalker.
| try: | ||
| # Allow NVDAObjects to redirect focus events to another object of their choosing. | ||
| if eventName=="gainFocus" and obj.focusRedirect: | ||
| obj=obj.focusRedirect |
There was a problem hiding this comment.
focusRedirect is used in the powerpnt appModule. We might have to make sure that this does not break. Having said that, I really like this being handled on the events level!
…at has a focusRedirect property as this is now handled in eventHandler.
|
I have removed the event_gainFocus from that Powerpoint NVDAObject class
as all it was doing was bouncing focus to the selection exactly the same
way as a focusRedirect. I have tested that this works as expected.
Note that focusRedirect was introduced a while back but only affected
NVDAObject.objectWithFocus() I.e. when NVDA had to correct back to the
system focus after exiting a menu etc.
Now of course it also redirects the gainFocus event itself from very
early on.
|
LeonarddeR
left a comment
There was a problem hiding this comment.
I found the following issue in Firefox:
- Explicitly disable browse mode with NVDA+space
- Expand and collapse a combo box
Expected: Browse mode stays off
Actual: Browse mode enables after collapsing the combo box.
Also, I wonder whether it would make sense to have the activate script expand a combo box (for example when pressing space on a combo box when in browse mode).
…hen in focus mode.
|
@LeonarddeR: I fixed the issue you found with NVDA switching back to browseMode after collapsing a combo box in focus mode. |
|
Your latter point makes sense indeed.
|
|
Is there anything else I have to address?
|
|
Sorry it took me some time to get back to this. I'm now observing the following in Firefox:
Expected: NVDA stays in focus mode and moves to the control in tab order after the combo box. This is how Edge and IE work. Apart from that, the code looks ok. |
|
It seems that this is a deliberate choice by Firefox and Chrome to
simply collapse the combo box when tab is pressed.
I don't really believe we should work around this one.
|
LeonarddeR
left a comment
There was a problem hiding this comment.
IN that case, I agree.
1. If the first child of a combo box is a text box, render that text box in browse mode. 2. Previously, we treated anything inside a combo box as being outside a browse mode document (nvaccess#8687). Don't do this for text boxes. Otherwise, you can't switch back to browse mode when the text box has focus.
1. If the first child of a combo box is a text box, render that text box in browse mode. 2. Previously, we treated anything inside a combo box as being outside a browse mode document (nvaccess#8687). Don't do this for text boxes. Otherwise, you can't switch back to browse mode when the text box has focus.
1. If the first child of a combo box is a text box, render that text box in browse mode. 2. Previously, we treated anything inside a combo box as being outside a browse mode document (nvaccess#8687). Don't do this for text boxes. Otherwise, you can't switch back to browse mode when the text box has focus.
1. If the first child of a combo box is a text box, render that text box in browse mode. 2. Previously, we treated anything inside a combo box as being outside a browse mode document (nvaccess#8687). Don't do this for text boxes. Otherwise, you can't switch back to browse mode when the text box has focus.
1. If the first child of a combo box is a text box, render that text box in browse mode. 2. Previously, we treated anything inside a combo box as being outside a browse mode document (#8687). Don't do this for text boxes. Otherwise, you can't switch back to browse mode when the text box has focus.
Link to issue number:
Fixes #8664
Summary of the issue:
Currently in NVDA, the user experience when using combo boxes on the web significantly differs by browser, plus some of the original code written to handle combo boxes has now broken in multiple browsers.
For example:
*In Firefox, collapsing a combo box works, but browseMode is not re-enabled.
Description of how this pull request fixes the issue:
This PR standardizes the way we handle combo boxes in browseMode, so that:
This PR also partly works around a bug in Chrome 68 and below where focusing a collapsed combo box focuses the active list item within the combo box. This has been fixed in Chrome 70.
This PR also fixes an issue in Edge where list items in the combo box were being treated were being treated as part of the document but with a broken textRange.
Testing performed:
Tested a standard HTML select (size=1) in Firefox 52, Firefox Nightly, Chrome 68 and Chrome 70.
Arrowed to the combo box in browse mode. Pressed alt+downArrow. Arrowed through the list. And closed the combo box with either enter, escape or alt+upArrow.
Known issues with pull request:
Combo boxes in Chrome 68 and below are still not perfect, but no worse than what they were before this PR.
Change log entry:
Bug fixes: