Include read only edit fields when navigating with quicknav commands #4164#6158
Conversation
|
Would it be possible to include this in 2016.4? |
|
I’ll support this request. It’s highly disconcerting not to find read only edit fields when using quick navigation commands. |
|
Patience, people. :) We're getting to reviews when we can, which is somewhat faster than previously. Thanks for your work.
|
| ) | ||
| return UIAControlQuicknavIterator(nodeType,self,pos,condition,direction) | ||
| elif nodeType=="nonTextContainer": | ||
| condition=createUIAMultiPropertyCondition({UIAHandler.UIA_ControlTypePropertyId:UIAHandler.UIA_ListControlTypeId,UIAHandler.UIA_IsKeyboardFocusablePropertyId:True},{UIAHandler.UIA_ControlTypePropertyId:UIAHandler.UIA_ComboBoxControlTypeId}) |
There was a problem hiding this comment.
This Edge change is going to cause a merge conflict with #6271 (which is not yet in master but will be once it's done incubating). Also, including read-only edits in Edge will cause problems with file upload controls because they expose a read-only edit, but they aren't included in the text pattern. For now, please revert the Edge change and file a follow up for Edge.
| attrs=[ | ||
| {"IAccessible::role":[oleacc.ROLE_SYSTEM_PUSHBUTTON,oleacc.ROLE_SYSTEM_RADIOBUTTON,oleacc.ROLE_SYSTEM_CHECKBUTTON,oleacc.ROLE_SYSTEM_COMBOBOX,oleacc.ROLE_SYSTEM_OUTLINE,oleacc.ROLE_SYSTEM_TEXT],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_READONLY:[None]}, | ||
| {"IAccessible::role":[oleacc.ROLE_SYSTEM_PUSHBUTTON,oleacc.ROLE_SYSTEM_RADIOBUTTON,oleacc.ROLE_SYSTEM_CHECKBUTTON,oleacc.ROLE_SYSTEM_OUTLINE],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_READONLY:[None]}, | ||
| {"IAccessible::role":[oleacc.ROLE_SYSTEM_COMBOBOX,oleacc.ROLE_SYSTEM_TEXT]}, |
There was a problem hiding this comment.
Just to clarify, are you not requiring read-only on combo boxes because you are accounting for read-only, editable ARIA combo boxes?
There was a problem hiding this comment.
Yes, though the main reason was that in the combobox code around line 263, there's no check for read only on combo boxes either.
| attrs={"IAccessible::role":[oleacc.ROLE_SYSTEM_PUSHBUTTON]} | ||
| elif nodeType=="edit": | ||
| attrs={"IAccessible::role":[oleacc.ROLE_SYSTEM_TEXT,oleacc.ROLE_SYSTEM_COMBOBOX],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_READONLY:[None],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_FOCUSABLE:[1],"IHTMLElement::%s"%"isContentEditable":[1]} | ||
| attrs={"IAccessible::role":[oleacc.ROLE_SYSTEM_TEXT,oleacc.ROLE_SYSTEM_COMBOBOX],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_FOCUSABLE:[1]} |
There was a problem hiding this comment.
You've removed the isContentEditable check here. That's important because otherwise, this will match non-editable combo boxes. That said, you can just do "IHTMLElement::isContentEditable" rather than that weird superfluous percent interpolation we had there for some unknown reason. :)
There was a problem hiding this comment.
Good one, fixed that.
| attrs={"IAccessible::role":[oleacc.ROLE_SYSTEM_PUSHBUTTON,oleacc.ROLE_SYSTEM_BUTTONMENU,oleacc.ROLE_SYSTEM_RADIOBUTTON,oleacc.ROLE_SYSTEM_CHECKBUTTON,oleacc.ROLE_SYSTEM_COMBOBOX,oleacc.ROLE_SYSTEM_LIST,oleacc.ROLE_SYSTEM_OUTLINE,oleacc.ROLE_SYSTEM_TEXT,IAccessibleHandler.IA2_ROLE_TOGGLE_BUTTON],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_READONLY:[None]} | ||
| attrs=[ | ||
| {"IAccessible::role":[oleacc.ROLE_SYSTEM_PUSHBUTTON,oleacc.ROLE_SYSTEM_BUTTONMENU,oleacc.ROLE_SYSTEM_RADIOBUTTON,oleacc.ROLE_SYSTEM_CHECKBUTTON,oleacc.ROLE_SYSTEM_LIST,oleacc.ROLE_SYSTEM_OUTLINE,IAccessibleHandler.IA2_ROLE_TOGGLE_BUTTON],"IAccessible::state_%s"%oleacc.STATE_SYSTEM_READONLY:[None]}, | ||
| {"IAccessible::role":[oleacc.ROLE_SYSTEM_COMBOBOX,oleacc.ROLE_SYSTEM_TEXT]}, |
There was a problem hiding this comment.
In Mozilla, for historical reasons, read-only "text" objects can be text leaf nodes (not editable text). Normally, these aren't rendered into the buffer, but they are in specific cases; see #1373. I'm concerned that in these cases, this code will match those. I think you need to check for the editable state here. That also means you'll need to put combo box in the rule above, since otherwise, we won't match non-editable combo boxes.
There was a problem hiding this comment.
Alright, let me know whether I fixed it correctly.
|
Actually, I need you to revert your copyright change to edge.py. Aside from no longer being valid, it's also causing a merge conflict when I try to incubate. :) Thanks. |
|
This pull requests allows one to navigate to read only edit fields with both e and f quick navigation keys in browse mode for Mozilla Gecko and MSHTML controls.
Fixes #4164.