We should remove the last uses of tree-based autocomplete, for example in the preferences window, so we can remove its implementation from the tree.

Once bug 1284391 is fixed, I think the preferences window is the only consumer left of the autogenerated richlistbox autocomplete in mozilla-central, however I kept the generic code we currently use to create the panel.

why is tags not autogenerated? Just for ac-site-icon?

Yep, I don't know of another way to assign an ID to the panel that we can use for styling it.

Comment on attachment 8939093 [details] Bug 1427366 - Use richlistbox autocomplete by default. https://reviewboard.mozilla.org/r/209512/#review221666 with the premise that the code is just wrong and these things should be part of urlbarbinding, not of the basic ac (so anything we do is "wrong" from an ideal point of view); I wonder if we could invert the rule, display:none these things by default, and show them only where it matters. In the end I think the only one using all of these is the urlbar. Thus I'd probably avoid "autogenerated", make it the default style, and add urlbar-only rules to fix it (those can use css selectors to apply only the urlbar without the need for an attribute). Would this be crazy or hard?

(In reply to Marco Bonardo [::mak] from comment #6) > Thus I'd probably avoid "autogenerated", make it the default style, and add > urlbar-only rules to fix it (those can use css selectors to apply only the > urlbar without the need for an attribute). > Would this be crazy or hard? That's just laborious, there are quite a few sites to audit and re-test after the change, including in-content: https://dxr.mozilla.org/mozilla-central/search?q=.ac-&redirect=true Maybe we can do this in a follow-up?

(In reply to :Paolo Amadini from comment #7) > That's just laborious, there are quite a few sites to audit and re-test > after the change, including in-content: Is it not actually the same for the current change? It is touching the same widgets, and thus those same widgets need to be tested. The main difference from your patch would be the removal of [autogenerated]. For most autocomplete fields it would work the same as your patch, since autocomplete.xml sets autogenerated = true by default in your patch. Afaik, the only widgets showing an icon are the urlbar, the search boxes and the homepage in about:preferences (though I'm not sure what the new autofill does). The only one that overrides "popup" (where you set autogenerated = true) looks like the search widget. But maybe I'm over-simplifying the thing, you looked into it with more depth. The list is a little bit shorter btw, since you are also including ac- things that won't change: https://searchfox.org/mozilla-central/search?q=%5C.ac-(type-icon%7Ctags%7Csepatator%7Curl%7Caction)&case=false&regexp=true&path= Alternatively, we could just file a follow-up to do the right thing, that is to remove these boxes completely from autocomplete.xml and move them to urlbarbindings.xml. I just fear it won't actually ever happen, for lack of resources, and this patch as-is will add some technical debt we'll never be able to pay. For the tags autocomplete differences, maybe we could reuse autocompletepopup="PopupAutoComplete"? There is some rules overlapping here, https://searchfox.org/mozilla-central/rev/11d0ff9f36465ce19b0c43d1ecc3025791eeb808/browser/base/content/browser.css#625 and https://searchfox.org/mozilla-central/rev/11d0ff9f36465ce19b0c43d1ecc3025791eeb808/toolkit/themes/shared/extensions/extensions.inc.css#1078. This again looks like it should be the default, and consumers should set ac-site-icon to display: -moz-box when they need it :( The whole thing is a mess.

(In reply to Marco Bonardo [::mak] from comment #8) > (In reply to :Paolo Amadini from comment #7) > > That's just laborious, there are quite a few sites to audit and re-test > > after the change, including in-content: > > Is it not actually the same for the current change? It is touching the same > widgets, and thus those same widgets need to be tested. Not exactly. Since we're not changing the default styles applied to panels that are not autogenerated, it's less likely we'll break any of them, like the in-content form autofill and the address bar, so we don't need to test them extensively. The reason I'd do this in a follow-up is that this patch blocks a number of other binding removals. In the follow-up I don't think we have to remove the boxes, we can just make them hidden as you suggest and remove the "autogenerated" style. It's roughly the same amount of work, but without blocking the XBL removals.

Comment on attachment 8939093 [details] Bug 1427366 - Use richlistbox autocomplete by default. https://reviewboard.mozilla.org/r/209512/#review222738 Ok, please file a follow-up bug for the removal of autogenerated, with as much details as possible. I still think this bug may end up increasing our technical debt, bug I don't want to block the binding removals momentum. Though, you didn't answer my question about using autocompletepopup="PopupAutoComplete" for tags instead of defining separate rules.

(In reply to Marco Bonardo [::mak] from comment #10) > Though, you didn't answer my question about using > autocompletepopup="PopupAutoComplete" for tags instead of defining separate > rules. Sorry, I missed the question. I'll look into this and see if PopupAutoComplete can be easily reused.

Comment on attachment 8939093 [details] Bug 1427366 - Use richlistbox autocomplete by default. https://reviewboard.mozilla.org/r/209512/#review223566 The changes look good to me, though Try is showing an a11y failure that looks related to this change

Comment on attachment 8939093 [details] Bug 1427366 - Use richlistbox autocomplete by default. https://reviewboard.mozilla.org/r/209512/#review224224 Thanks

Comment on attachment 8939093 [details] Bug 1427366 - Use richlistbox autocomplete by default. https://reviewboard.mozilla.org/r/209512/#review224314 ::: accessible/tests/mochitest/tree/test_txtctrl.xul:138 (Diff revision 5) > var txc = document.getElementById("txc_autocomplete"); > SimpleTest.ok(txc, "Testing (New) Toolkit autocomplete widget."); > > // Dumb access to trigger popup lazy creation. > dump("Trigget popup lazy creation"); > waitForEvent(EVENT_REORDER, txc, test_AutocompleteControl); it'd be nice to inline the function like () => { testAccessibleTree("txc_autocomplete", accTree); SimpleTest.finish(); };

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/63c152ad62b0 Use richlistbox autocomplete by default. r=mak,surkov

Marco, will TB be affected by this?

(In reply to Jorg K (GMT+1) from comment #22) > Marco, will TB be affected by this? It's likely, you may want to check your autocomplete fields and verify they still appear correctly. Otherwise it may need some css adjustments.

Thanks Marco, yes, this needs some CSS changes, bug 1436764.