From https://bugzilla.mozilla.org/show_bug.cgi?id=1431246#c2: > Neil, is it possible we could remove the "thumb" binding at > https://searchfox.org/mozilla-central/source/toolkit/content/widgets/ > scrollbar.xml#12? It's bound to the "thumb" element at > https://searchfox.org/mozilla-central/source/toolkit/content/minimal-xul. > css#91. I'm not sure exactly what `extends="xul:button"` is doing - is this > something we could delete and/or move into XUL <thumb> directly instead of > through XBL? And https://bugzilla.mozilla.org/show_bug.cgi?id=1431246#c3: > That line causes the element to get a nsButtonBoxFrame instead of nsBoxFrame. > > But you should be able to easily remove the need for a binding by adding a > mapping for <thumb> in nsCSSFrameConstructor::FindXULTagData, and also > adding to the whitelist at nsXBLPrototypeBinding::CheckTagNameWhiteList

Neil, does the `display: -moz-box !important` on thumb https://dxr.mozilla.org/mozilla-central/source/toolkit/content/minimal-xul.css#92 remove the need to set up the mapping for nsButtonBoxFrame in nsCSSFrameConstructor::FindXULTagData?

No, that just gives it a box frame, which is generally redundant since it should already be one. It looks like the display line was added by 248407 to fix a crash.

> But you should be able to easily remove the need for a binding by adding a > mapping for <thumb> in nsCSSFrameConstructor::FindXULTagData, and also > adding to the whitelist at nsXBLPrototypeBinding::CheckTagNameWhiteList I've looked at nsXBLPrototypeBinding::CheckTagNameWhiteList and it's not clear to me why we would need to whitelist thumb here. That appears to be used for the 'display' or 'extends' attributes on the binding, and I don't see anything that uses display='xul:thumb' or extends='xul:thumb' (https://dxr.mozilla.org/mozilla-central/search?q=display%3D%22xul&redirect=true / https://dxr.mozilla.org/mozilla-central/search?q=extends%3D%22xul&redirect=false). The only non-tag hit for "xul:thumb" has to do with accesibility roles, which shouldn't be affected by that: https://dxr.mozilla.org/mozilla-central/search?q=xul%3Athumb&redirect=true. It's a bit odd that the thumb itself doesn't have that role, but doesn't seem like removing the thumb binding would change anything there.

(In reply to Brian Grinstead [:bgrins] from comment #3) > > But you should be able to easily remove the need for a binding by adding a > > mapping for <thumb> in nsCSSFrameConstructor::FindXULTagData, and also > > adding to the whitelist at nsXBLPrototypeBinding::CheckTagNameWhiteList > Yes, I realize that you don't need to add it in this case. Adding to FindXULTagData should suffice.

I'm not sure if the change to FindXULTagData is right - I just copied what was there for `button`.

Comment on attachment 8943979 [details] Bug 1431522 - Remove the 'thumb' binding;+6102 Somehow the reviewer field didn't get set in mozreview

Comment on attachment 8943979 [details] Bug 1431522 - Remove the 'thumb' binding;+6102 https://reviewboard.mozilla.org/r/214304/#review229552

Rebased - review flag somehow didn't get carried over but I think mozreview still sees it as an r+ since the land commits button is active. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee37791e04a98f9d7f87f2b435817601be970bd4

I am pretty sure when this lands the scalethumb binding can be removed as well (in bug 1440393).

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #12) > I am pretty sure when this lands the scalethumb binding can be removed as > well (in bug 1440393). Yeah - although it looks like we'll also probably need to import scale.css into components.css and remove it out of all <resources> in scale.xml.

Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a20647d8a887 Remove the 'thumb' binding;r=enndeakin+6102+6102