Closed
Bug 1428930
Opened 6 years ago
Closed 6 years ago
Move as many XBL accessibility roles into XULMap.h as possible
Categories
(Toolkit :: UI Widgets, task)
Toolkit
UI Widgets
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: bgrins, Assigned: Paolo)
References
(Blocks 1 open bug)
Details
(Whiteboard: [xbl-available])
Attachments
(1 file)
As of Bug 1403231 we have a way to declare roles on a tag name as an alternative to doing it on a XBL binding: https://dxr.mozilla.org/mozilla-central/source/accessible/base/XULMap.h. So far we've moved these one-by-one as we tackle the individual bindings, but if we front-load that work we can continue to use artifact builds for the individual binding removals (which often don't require other platform changes). See the ~40 bindings using the [role] feature here: https://bgrins.github.io/xbl-analysis/tree/#role. There may be cases where the binding can't be keyed off a tag name only (since XBL bindings can be attached to any arbitrary CSS selector). In that case, we could either not include them in this bug or add necessary XULMap features to handle these cases (like allowing attribute selectors in the mapping). I'd prefer to do the former for this bug to keep things simple, and we can handle the tougher cases in a follow up.
Reporter | ||
Comment 1•6 years ago
|
||
It may be the case that we can only easily do this with 'leaf' bindings that don't have any children that extend them. For example "button-base" has a role of "xul:button" but nothing ever directly creates button-base, the role is surely carried through to extended bindings that don't define their own role: button-base [role="xul:button"] button button-repeat ctrlTab-preview menu-button-base download-subview-toolbarbutton menu-button To move the role off of button I think we'd need to capture every tag of all children (which may or may not be difficult depending on the case). That said, there are plenty of cases where it'll be easier.
Assignee | ||
Comment 2•6 years ago
|
||
I'm looking into this.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•6 years ago
|
||
I'm building a list of bindings and associated CSS selectors to find out which groups always apply to the same tag. Since there is a limited number of bindings to go through, I'm doing this manually with DXR searches and I'm cross-checking the results with the list in <https://bgrins.github.io/xbl-analysis/tree/#role>. In doing this, I've noticed that the latter doesn't report bindings with the same name in different files, for example only one of these bindings is shown: https://dxr.mozilla.org/mozilla-central/search?q=%23menu%22
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 4•6 years ago
|
||
I found at least one case where a custom binding that should probably inherit from a base one doesn't: https://dxr.mozilla.org/mozilla-central/rev/c38d22170d6f27e94c3c53093215d20255fab27a/browser/components/preferences/handlers.css#13-15 My guess is that in this case the richlistitem doesn't get a role assigned, although it should. Assigning a role to all richlistitem tags would fix this. While we should in theory search the CSS selectors for all bindings and see if there are more cases like this, I don't think that it matters in practice, as we would be most likely fixing existing bugs by assigning a role where there wasn't one.
Assignee | ||
Comment 5•6 years ago
|
||
Here are the bindings with an explicit XUL role attached, listed together with the ones that inherit from them without overriding the role. Each entry includes the CSS selectors used to apply the binding, which in some cases are also platform-specific or inside media queries. I've excluded the "browser", "editor", and "iframe" bindings that have role="outerdoc". Another special case is "menu-vertical", which is unused in mozilla-central but still referenced in comm-central. browser/components/customizableui/content/toolbar.xml#toolbar role="xul:toolbar" toolbar[customizable="true"] browser/components/customizableui/content/toolbar.xml#toolbar-drag #nav-bar #TabsToolbar:not(:-moz-lwtheme) #toolbar-menubar:not([autohide="true"]):not(:-moz-lwtheme) browser/components/customizableui/content/toolbar.xml#toolbar-menubar-autohide #toolbar-menubar[autohide="true"] toolbar[type="menubar"][autohide="true"] browser/components/translation/translation-infobar.xml#translationbar role="xul:alert" notification[value="translation"] toolkit/content/widgets/autocomplete.xml#autocomplete role="xul:combobox" textbox[type="autocomplete"] browser/base/content/urlbarBindings.xml#urlbar #urlbar browser/components/search/content/search.xml#searchbar-textbox .searchbar-textbox toolkit/content/widgets/autocomplete.xml#autocomplete-base-popup role="none" toolkit/content/widgets/autocomplete.xml#autocomplete-result-popup panel[type="autocomplete"] browser/components/search/content/search.xml#browser-search-autocomplete-result-popup #PopupSearchAutoComplete toolkit/content/widgets/autocomplete.xml#autocomplete-rich-result-popup panel[type="autocomplete-richlistbox"] browser/base/content/urlbarBindings.xml#urlbar-rich-result-popup #PopupAutoCompleteRichResult toolkit/content/widgets/button.xml#button-base role="xul:button" browser/base/content/browser-tabPreviews.xml#ctrlTab-preview .ctrlTab-preview toolkit/content/widgets/button.xml#button button toolkit/content/widgets/button.xml#button-repeat button[type="repeat"] toolkit/content/widgets/button.xml#menu button[type="menu"] button[type="panel"] toolkit/content/widgets/button.xml#menu-button-base browser/components/downloads/content/download.xml#download-subview-toolbarbutton .subviewbutton.download toolkit/content/widgets/button.xml#menu-button button[type="menu-button"] toolkit/content/widgets/toolbarbutton.xml#menu-button toolbarbutton[type="menu-button"] toolkit/content/widgets/checkbox.xml#checkbox role="xul:checkbox" checkbox toolkit/content/widgets/colorpicker.xml#colorpickertile role="xul:colorpickertile" .colorpickertile toolkit/content/widgets/colorpicker.xml#colorpicker-button role="xul:colorpicker" colorpicker[type="button"] toolkit/content/widgets/general.xml#dropmarker role="xul:dropmarker" dropmarker toolkit/content/widgets/groupbox.xml#groupbox role="xul:groupbox" groupbox toolkit/content/widgets/listbox.xml#listbox-base role="xul:listbox" toolkit/content/widgets/listbox.xml#listbox listbox toolkit/content/widgets/richlistbox.xml#richlistbox richlistbox toolkit/content/widgets/autocomplete.xml#autocomplete-richlistbox .autocomplete-richlistbox toolkit/mozapps/extensions/content/extensions.xml#categories-list #categories toolkit/content/widgets/listbox.xml#listcell role="xul:listcell" listcell toolkit/content/widgets/listbox.xml#listcell-checkbox listcell[type="checkbox"] toolkit/content/widgets/listbox.xml#listcell-checkbox-iconic listitem[type="checkbox"].listitem-iconic toolkit/content/widgets/listbox.xml#listcell-iconic .listcell-iconic toolkit/content/widgets/listbox.xml#listhead role="xul:listhead" listhead toolkit/content/widgets/listbox.xml#listheader role="xul:listheader" listheader toolkit/content/widgets/listbox.xml#listitem role="xul:listitem" listitem browser/components/preferences/handlers.xml#offlineapp listitem.offlineapp toolkit/content/widgets/listbox.xml#listitem-checkbox listitem[type="checkbox"] toolkit/content/widgets/listbox.xml#listitem-checkbox-iconic listitem[type="checkbox"].listitem-iconic toolkit/content/widgets/listbox.xml#listitem-iconic .listitem-iconic toolkit/content/widgets/richlistbox.xml#richlistitem richlistitem browser/base/content/pageinfo/feeds.xml#feed richlistitem[feed] browser/components/downloads/content/download.xml#download richlistitem[type="download"] richlistitem.download[active] browser/components/preferences/handlers.xml#handler-base browser/components/preferences/handlers.xml#handler #handlersView > richlistitem browser/components/preferences/handlers.xml#handler-selected #handlersView > richlistitem[selected="true"] browser/components/preferences/siteListItem.xml#siteListItem #sitesList > richlistitem browser/extensions/formautofill/content/formautofill.xml#autocomplete-profile-listitem-base browser/extensions/formautofill/content/formautofill.xml#autocomplete-creditcard-insecure-field #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-insecureWarning"] browser/extensions/formautofill/content/formautofill.xml#autocomplete-profile-listitem #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"] browser/extensions/formautofill/content/formautofill.xml#autocomplete-profile-listitem-clear-button #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-clear-button"] browser/extensions/formautofill/content/formautofill.xml#autocomplete-profile-listitem-footer #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-footer"] toolkit/content/widgets/autocomplete.xml#autocomplete-richlistitem .autocomplete-richlistitem toolkit/content/widgets/autocomplete.xml#autocomplete-richlistitem-insecure-field #PopupAutoComplete > richlistbox > richlistitem[originaltype="insecureWarning"] toolkit/mozapps/extensions/content/extensions.xml#addon-base toolkit/mozapps/extensions/content/extensions.xml#addon-generic .addon[status="installed"] toolkit/mozapps/extensions/content/extensions.xml#addon-installing .addon[status="installing"] toolkit/mozapps/extensions/content/extensions.xml#addon-uninstalled .addon[pending="uninstall"] toolkit/mozapps/extensions/content/extensions.xml#category .category toolkit/mozapps/handling/content/handler.xml#handler richlistitem[type="handler"] toolkit/mozapps/update/content/updates.xml#update update toolkit/content/widgets/menu.xml#menuitem-base role="xul:menuitem" toolkit/content/widgets/menu.xml#menubutton-item menuitem.menuitem-non-iconic toolkit/content/widgets/menu.xml#menuitem menuitem toolkit/content/widgets/menu.xml#menuitem-iconic menuitem.menuitem-iconic menuitem[type="checkbox"] menuitem[type="radio"] toolkit/content/widgets/menu.xml#menuitem-iconic-desc-noaccel menuitem[description] toolkit/content/widgets/menu.xml#menuitem-iconic-noaccel menulist > menupopup > menuitem toolkit/content/widgets/menu.xml#menu-base browser/themes/linux/places/organizer.xml#toolbarbutton-dropdown #placesMenu > menu toolkit/content/widgets/menu.xml#menu menu toolkit/content/widgets/menu.xml#menucaption menucaption toolkit/content/widgets/menu.xml#menu-iconic menu.menu-iconic toolkit/content/widgets/menu.xml#menu-menubar menubar > menu toolkit/content/widgets/menu.xml#menu-menubar-iconic menubar > menu.menu-iconic toolkit/content/widgets/menulist.xml#menulist role="xul:menulist" menulist toolkit/content/widgets/menulist.xml#menulist-editable menulist[editable="true"] toolkit/content/widgets/menulist.xml#menulist-description menulist[type="description"] toolkit/content/widgets/menulist.xml#menulist-popuponly menulist[popuponly="true"] toolkit/content/widgets/notification.xml#notification role="xul:alert" notification toolkit/content/widgets/popup.xml#panel role="xul:panel" panel toolkit/content/widgets/popup.xml#arrowpanel panel[type="arrow"] toolkit/content/widgets/datetimepopup.xml#datetime-popup #DateTimePickerPanel[active="true"] toolkit/content/widgets/popup.xml#popup role="xul:menupopup" popup menupopup browser/base/content/tabbrowser.xml#tabbrowser-alltabs-popup #alltabs-popup .new-tab-popup browser/components/places/content/menu.xml#places-popup-base menupopup[placespopup="true"] browser/components/places/content/menu.xml#places-popup-arrow #BMB_bookmarksPopup toolkit/content/widgets/popup.xml#popup-scrollbars menulist > menupopup toolkit/content/widgets/popup.xml#tooltip role="xul:tooltip" tooltip toolkit/content/widgets/progressmeter.xml#progressmeter role="xul:progressmeter" progressmeter .downloadProgress[mode="undetermined"] toolkit/content/widgets/progressmeter.xml#progressmeter-undetermined progressmeter[mode="undetermined"] toolkit/content/widgets/radio.xml#radio role="xul:radiobutton" radio toolkit/content/widgets/radio.xml#radiogroup role="xul:radiogroup" radiogroup toolkit/content/widgets/scale.xml#scale role="xul:scale" scale toolkit/content/widgets/videocontrols.xml#suppressChangeEvent .scrubber .volumeControl toolkit/content/widgets/scale.xml#scalethumb role="xul:thumb" .scale-thumb toolkit/content/widgets/tabbox.xml#tab role="xul:tab" tab browser/base/content/tabbrowser.xml#tabbrowser-tab .tabbrowser-tab toolkit/content/widgets/tabbox.xml#tabpanels role="xul:tabpanels" tabpanels browser/base/content/tabbrowser.xml#tabbrowser-tabpanels .tabbrowser-tabpanels toolkit/content/widgets/tabbox.xml#tabs role="xul:tabs" tabs browser/base/content/tabbrowser.xml#tabbrowser-tabs #tabbrowser-tabs toolkit/content/widgets/text.xml#text-base role="xul:text" description toolkit/content/widgets/text.xml#text-label label toolkit/content/widgets/text.xml#label-control label[control] label.radio-label label.checkbox-label label.toolbarbutton-multiline-text toolkit/content/widgets/text.xml#text-link role="xul:link" label.text-link label[onclick] toolkit/content/widgets/textbox.xml#textbox role="xul:textbox" textbox toolkit/content/widgets/findbar.xml#findbar-textbox .findbar-textbox toolkit/content/widgets/numberbox.xml#numberbox textbox[type="number"] toolkit/content/widgets/textbox.xml#search-textbox textbox[type="search"] toolkit/content/widgets/textbox.xml#textarea textbox[multiline="true"] toolkit/content/widgets/toolbar.xml#menubar role="xul:menubar" menubar toolkit/content/widgets/toolbar.xml#toolbar role="xul:toolbar" toolbar toolkit/components/printing/content/printPreviewBindings.xml#printpreviewtoolbar toolbar[printpreview="true"] toolkit/content/widgets/findbar.xml#findbar findbar toolkit/content/widgets/toolbar.xml#toolbar-drag #placesToolbar toolbar[type="menubar"]:not([autohide="true"]):not(:-moz-lwtheme) toolkit/content/widgets/toolbar.xml#toolbar-menubar-autohide #toolbar-menubar[autohide="true"] toolbar[type="menubar"][autohide="true"] toolkit/content/widgets/toolbar.xml#toolbardecoration role="xul:toolbarseparator" toolbarseparator toolbarspacer toolbarspring toolkit/content/widgets/toolbarbutton.xml#toolbarbutton role="xul:toolbarbutton" toolbarbutton toolkit/content/widgets/toolbarbutton.xml#menu toolbarbutton[type="menu"] toolbarbutton[type="panel"] toolkit/content/widgets/toolbarbutton.xml#toolbarbutton-image browser/base/content/tabbrowser.xml#tabbrowser-close-tab-button .tab-close-button toolkit/content/widgets/toolbarbutton.xml#toolbarbutton-badged toolbarbutton.badged-button toolbarbutton.badged-button > toolbarbutton toolkit/content/widgets/toolbarbutton.xml#toolbarbutton-badged-menu toolbarbutton.badged-button[type="menu"] toolbarbutton.badged-button[type="panel"] toolkit/content/widgets/tree.xml#columnpicker role="xul:button" treecolpicker toolkit/content/widgets/tree.xml#tree role="xul:tree" tree browser/components/places/content/tree.xml#places-tree tree[type="places"] toolkit/content/widgets/autocomplete.xml#autocomplete-tree .autocomplete-tree toolkit/content/widgets/tree.xml#treecol-base role="xul:treecolumnitem" toolkit/content/widgets/tree.xml#treecol treecol toolkit/content/widgets/tree.xml#treecol-image treecol.treecol-image toolkit/content/widgets/tree.xml#treecols role="xul:treecolumns" treecols
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to :Paolo Amadini from comment #3) > I'm building a list of bindings and associated CSS selectors to find out > which groups always apply to the same tag. > > Since there is a limited number of bindings to go through, I'm doing this > manually with DXR searches and I'm cross-checking the results with the list > in <https://bgrins.github.io/xbl-analysis/tree/#role>. In doing this, I've > noticed that the latter doesn't report bindings with the same name in > different files, for example only one of these bindings is shown: > > https://dxr.mozilla.org/mozilla-central/search?q=%23menu%22 Thanks, this was an issue with some other bindings and I just had missed this menu case. Right now I fix this by prefixing all bindings in certain files with another string (https://github.com/bgrins/xbl-analysis/blob/0b2b868c1ac84a40eab4fe89720bd8e3067bead2/scripts/xbl-files.js#L201-L217). For this case I should probably change the id of just the 'menu' binding in those files.
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 7•6 years ago
|
||
I made a list of the simpler cases that can be handled just by adding tags to XULMap, but I won't be able to use full builds for the next two weeks, so feel free to take the bug in the meantime. It would also be nice to fix bug 1414230 before adding all the constructor functions to the separate file, but not required. The richlistitem tag is a special case that I think is simple enough to be handled in this bug. We should add back role="xul:listitem" to the "toolkit/mozapps/update/content/updates.xml#update" binding, since it's bound to a custom "update" tag instead of a richlistitem. Alternatively we can change the code to use a class instead of a custom tag, although the latter may need more testing. Here are the bindings whose role I'm confident can be removed and added back to tags: browser/components/customizableui/content/toolbar.xml#toolbar role="xul:toolbar" toolkit/content/widgets/toolbar.xml#toolbar role="xul:toolbar" toolbar findbar browser/components/translation/translation-infobar.xml#translationbar role="xul:alert" toolkit/content/widgets/notification.xml#notification role="xul:alert" notification toolkit/content/widgets/checkbox.xml#checkbox role="xul:checkbox" checkbox toolkit/content/widgets/general.xml#dropmarker role="xul:dropmarker" dropmarker toolkit/content/widgets/groupbox.xml#groupbox role="xul:groupbox" groupbox toolkit/content/widgets/listbox.xml#listbox-base role="xul:listbox" listbox richlistbox toolkit/content/widgets/listbox.xml#listcell role="xul:listcell" listcell toolkit/content/widgets/listbox.xml#listhead role="xul:listhead" listhead toolkit/content/widgets/listbox.xml#listheader role="xul:listheader" listheader toolkit/content/widgets/listbox.xml#listitem role="xul:listitem" listitem richlistitem toolkit/content/widgets/menu.xml#menuitem-base role="xul:menuitem" menuitem menu menucaption toolkit/content/widgets/menulist.xml#menulist role="xul:menulist" menulist toolkit/content/widgets/popup.xml#popup role="xul:menupopup" popup menupopup toolkit/content/widgets/popup.xml#tooltip role="xul:tooltip" tooltip toolkit/content/widgets/progressmeter.xml#progressmeter role="xul:progressmeter" progressmeter toolkit/content/widgets/radio.xml#radio role="xul:radiobutton" radio toolkit/content/widgets/radio.xml#radiogroup role="xul:radiogroup" radiogroup toolkit/content/widgets/tabbox.xml#tab role="xul:tab" tab toolkit/content/widgets/tabbox.xml#tabpanels role="xul:tabpanels" tabpanels toolkit/content/widgets/tabbox.xml#tabs role="xul:tabs" tabs toolkit/content/widgets/toolbar.xml#menubar role="xul:menubar" menubar toolkit/content/widgets/toolbar.xml#toolbardecoration role="xul:toolbarseparator" toolbarseparator toolbarspacer toolbarspring toolkit/content/widgets/tree.xml#columnpicker role="xul:button" treecolpicker toolkit/content/widgets/tree.xml#tree role="xul:tree" tree toolkit/content/widgets/tree.xml#treecol-base role="xul:treecolumnitem" treecol toolkit/content/widgets/tree.xml#treecols role="xul:treecolumns" treecols
Reporter | ||
Updated•6 years ago
|
Whiteboard: [xbl-available]
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Comment 8•6 years ago
|
||
Is this bug simply involved looking at comment 7 and add them into XULMap.h and nsAccessibilityService.cpp like what :bgrins did in bug 1422386? https://hg.mozilla.org/try/rev/8b6f96efe45b915f28bc6c89c29b60bad598abc0#l1.9 If so I can provide a drive-by fix so we could unblock a few more things, and for you to work on other things in parallel.
Flags: needinfo?(paolo.mozmail)
Comment 9•6 years ago
|
||
(Not exactly drive-by, it would still take some time to test the patch out and stuff, so don't worry about it if you already have an WIP ready)
Assignee | ||
Comment 10•6 years ago
|
||
Yes, this bug is simple to implement but I was already working on it. I fixed bug 1414230 first to make it easier to create the table, then I addressed all the bindings in comment 7 minus the "popup" and "menupopup" tags because I found out there's an explicit lookup for their XBL role in XULButtonAccessible::IsAcceptableChild. You can look at bug 1430777 if you want to convert this or some of the other more complex cases. The conversions don't need to happen all at the same time, you can file new bugs and tackle them a few at a time.
Flags: needinfo?(paolo.mozmail)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•6 years ago
|
||
To make the process simpler, I didn't actually remove any binding even when they were only setting the role. There are already bugs on file for these cases, like bug 1422386. https://treeherder.mozilla.org/#/jobs?repo=try&revision=30b9ab947aba75cd52641814c9ccf39cba82dbac
Reporter | ||
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8948981 [details] Bug 1428930 - Move some XBL accessibility roles into XULMap.h. https://reviewboard.mozilla.org/r/218396/#review224304 Thanks! This looks great to me on the XBL side
Attachment #8948981 -
Flags: review?(bgrinstead) → review+
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8948981 [details] Bug 1428930 - Move some XBL accessibility roles into XULMap.h. https://reviewboard.mozilla.org/r/218396/#review224302 XBL hierarchy is more complicated than I thought, r=me to the best of my knowledge and abilities to dig through bindings ::: accessible/base/XULMap.h:92 (Diff revision 1) > + > + RefPtr<nsTreeColumns> treeCols = treeFrame->Columns(); > + int32_t count = 0; > + treeCols->GetCount(&count); > + > + // Outline of list accessible. nit: it probably meant 'or' not 'of'
Attachment #8948981 -
Flags: review?(surkov.alexander) → review+
Comment 15•6 years ago
|
||
there's bunch of bindings in comm-central (outside mozilla folder), which will should be checked before landing the patch. Paolo, is it something you could take care of? [1] https://searchfox.org/comm-central/search?q=role%3D%22xul%3A&case=true®exp=false&path=
Flags: needinfo?(paolo.mozmail)
Assignee | ||
Comment 16•6 years ago
|
||
At present I've only noticed that the "panebutton" binding in "common/bindings/preferences.xml" may be affected by this patch. We didn't have to take care of this binding because we have already removed support for preferences windows entirely. A simple workaround can be to change the code so the panebutton is bound to a tag that already gets the "xul:listitem" role thanks to the new tag mapping, instead of a custom tag. I'll file a bug with this information.
Flags: needinfo?(paolo.mozmail)
Comment 17•6 years ago
|
||
Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/1185d23b7e3b Move some XBL accessibility roles into XULMap.h. r=bgrins,surkov
Comment 18•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1185d23b7e3b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•