AFAICT this interface is used only in one place (to change an accessibilty role for a richlistbox with a panel parent). I haven't been able to confirm that code ever runs, but if so this case should be able to be changed to check for the tag name or some other information about the parent node. If the interface is gone then we can remove the [implements] attribute on the bindings that use it, which should simplify replacing the XBL version of those bindings with Custom Elements. - https://searchfox.org/mozilla-central/search?q=XULPopupElement&redirect=false - https://searchfox.org/mozilla-central/rev/0b613c3887789f7786cd3131dfe9648398f4a6ac/accessible/xul/XULListboxAccessible.cpp#158-165

Alexander, can you help me figure out if the change to XULListboxAccessible is correct? Unfortunately this condition doesn't seem to be covered by automated tests, as the accessibility tests pass even with it commented out. And I'm not sure how to manually test this (or write a test to make sure it is working). The comment indicates that this is for the URL bar but I'm not seeing the code being called when opening the URL bar - do I need to somehow enable accessibility features?

XUL combobox tests are located here [1]. I'd be nice to add a new one there, if your case is not covered. However if we have test coverage for your case, then the reason, why the changes don't have any effect, could be [2]. [1] https://dxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/tree/test_combobox.xul?q=test_combobox.xul&redirect_type=direct [2] https://dxr.mozilla.org/mozilla-central/source/accessible/generic/Accessible.cpp?q=ARIATransformRole&redirect_type=direct#1463

(In reply to alexander :surkov from comment #4) > XUL combobox tests are located here [1]. I'd be nice to add a new one there, > if your case is not covered. However if we have test coverage for your case, > then the reason, why the changes don't have any effect, could be [2]. Huh, so does (2) indicate that this condition in XULListboxAccessible::NativeRole is dead code? I don't want to add a test for this unless if we can confirm this is actually used in the browser. If I add printf inside of that function it never gets run when I `./mach run` and open the URL bar - do I need to do something else to enable accessibility features?

(In reply to Brian Grinstead [:bgrins] from comment #5) > (In reply to alexander :surkov from comment #4) > > XUL combobox tests are located here [1]. I'd be nice to add a new one there, > > if your case is not covered. However if we have test coverage for your case, > > then the reason, why the changes don't have any effect, could be [2]. > > Huh, so does (2) indicate that this condition in > XULListboxAccessible::NativeRole is dead code? probably not, there's a mutlti column list case there, see roles::TREE in XULListboxAccessible::NativeRole() > I don't want to add a test > for this unless if we can confirm this is actually used in the browser. If I > add printf inside of that function it never gets run when I `./mach run` and > open the URL bar - do I need to do something else to enable accessibility > features? yes, you could start a screen reader, for example, VoiceOver on mac (CMD+f5), or invoke it via the browser console, like: Cc["@mozilla.org/accessibilityService;1"].getService(Ci.nsIAccessibilityService)

(In reply to alexander :surkov from comment #6) > > for this unless if we can confirm this is actually used in the browser. If I > > add printf inside of that function it never gets run when I `./mach run` and > > open the URL bar - do I need to do something else to enable accessibility > > features? > > yes, you could start a screen reader, for example, VoiceOver on mac > (CMD+f5), or invoke it via the browser console, like: > Cc["@mozilla.org/accessibilityService;1"].getService(Ci. > nsIAccessibilityService) OK, perfect - I can see the condition is getting run both with and without the patch once the service is started. I'll look into adding a test case in the combobox test.

Comment on attachment 8933865 [details] Bug 1422465 - Remove the nsIDOMXULPopupElement interface and [implements] attribute on the "popup-base" and "panel" bindings https://reviewboard.mozilla.org/r/204792/#review211278 This is going to break the code at https://searchfox.org/comm-central/source/suite/common/utilityOverlay.js#1354 because that does `instanceof Components.interfaces.nsIDOMXULPopupElement`. Please file a comm-central bug so they'll know to update to some other method of detecting popups (e.g. copy what browser/base/content/utilityOverlay.js does with explicit namespace/tagname tests). Nice to see the classinfo bits go away!

Comment on attachment 8934712 [details] Bug 1422465 - Add regression test to ensure that the awesomebar richlistbox gets the COMBOBOX_LIST role; https://reviewboard.mozilla.org/r/205608/#review211480 ::: accessible/tests/browser/general/browser.ini:8 (Diff revision 1) > !/accessible/tests/browser/shared-head.js > head.js > !/accessible/tests/mochitest/*.js > > [browser_test_doc_creation.js] > +[browser_test_popup_autocomplete.js] browser_test_urlbar.js would be a better match to better reflect we test browser UI there, otherwise it makes sense to replicate urlbar widget in the test

Comment on attachment 8933865 [details] Bug 1422465 - Remove the nsIDOMXULPopupElement interface and [implements] attribute on the "popup-base" and "panel" bindings https://reviewboard.mozilla.org/r/204792/#review211482

Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1355b86006a4 Add regression test to ensure that the awesomebar richlistbox gets the COMBOBOX_LIST role;r=surkov https://hg.mozilla.org/integration/autoland/rev/319fbe5fff76 Remove the nsIDOMXULPopupElement interface and [implements] attribute on the "popup-base" and "panel" bindings;r=bz,surkov

https://hg.mozilla.org/mozilla-central/rev/1355b86006a4 https://hg.mozilla.org/mozilla-central/rev/319fbe5fff76

How would I fix the following code /* * @param {nsIDOMXULPopupElement} target Current menupopup node */ function QpFolderMenu(target) { if (!(target instanceof CI.nsIDOMXULPopupElement)) { throw new Error('QpFolderMenu: Illegal argument "target" ' + target); } this throws Error: TypeError: invalid 'instanceof' operand CI.nsIDOMXULPopupElement is there an alternative interface?

There is not. What semantics do you actually want? That is, why do you care whether you have a nsIDOMXULPopupElement?