47 bytes, text/x-phabricator-request
|Details | Review|
This is intended to remove support for changing element tag names using XBL and removing nsBindingManager::ResolveTag and the relevant ilk. The relevant bits can be seen grepping for: extends="xul: (There's also extends="html: and svg, but those are only in tests).
(FWIW, this is already kinda broken, we apply this super inconsistently)
After the dependencies are fixed, it seems that the only elements left are buttons, see the selectors here: https://bgrins.github.io/xbl-analysis/tree/#display Basically, it seems that button[type="menu"] and toolbarbutton[type="menu"] should get a menu frame instead of a button frame. Emilio, is this something that can be implemented in nsCSSFrameConstructor::FindXULTagData? Brian, do you think we need to handle cases where this attribute changes, maybe because we set it after adding the element to the document? In this case, Emilio, is this something that is already handled, or would it need to be handled specially after you remove the platform code for "display" that you mentioned in comment 0?
It can be done there with a bit of effort to reframe when that attribute changes. I'm happy to write that patch. : https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/dom/xul/nsXULElement.cpp#1180
That being said, other than in tests there doesn't seem to be that many: https://searchfox.org/mozilla-central/search?q=button.*type%3D%22menu&case=false®exp=true&path=*.xul Are those something we could replace for actual menus easily? If we could avoid the special-case it'd be really neat.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4) > Are those something we could replace for actual menus easily? If we could > avoid the special-case it'd be really neat. This would also simplify some of the conversions to Custom Elements, so I'll look into this next.
It will be unused as soon as the dependent bugs land and the [type="menu"] bits mentioned in comment 2+ are fixed.
Jorg, if TB has any bindings that use the [display] / [extends] attribute on <binding> they will need to be migrated away from using it after this patch (which depends on a bit more work being done on the m-c side before it can land). It looks like this is used in some TB folders: https://searchfox.org/comm-central/search?q=display%3D&path=.xml. There are bugs blocking this one showing examples of migration - sometimes they can just be removed directly if it's just box/hbox or if it's an element we've already covered on the layout side (like toolbarpaletteitem was covered in Bug 1451256).
Thanks, I filed bug 1509139 for the TB work.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3) > It can be done there with a bit of effort to reframe when that attribute > changes. I'm happy to write that patch. > > : > https://searchfox.org/mozilla-central/rev/ > 55895c49f55073d82d977cb74ec1d3a71ae4b25f/dom/xul/nsXULElement.cpp#1180 Emilio, would you be able to write the patch for this in bug 1512993? After experimenting with removing the menu frame altogether from menu buttons, we're thinking of keeping it to reduce code duplication. The toolbarbutton[type="menu"] binding removals in my previous patch would still apply, since switching based on the "type" attribute would be handled without relying on XBL. (We do have code that changes the attribute at runtime, for example this is done in the new tab button when containers are enabled or disabled.) After this, bug 1507704 is probably the only remaining special case to get rid of before we can land this bug.
Attachment #9026781 - Attachment description: Bug 1450652 - Remove platform support for display="" in XBL bindings. → Bug 1450652 - Remove platform support for display="" in XBL bindings. r=mats
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/5edf9199407c Remove platform support for display="" in XBL bindings. r=mats
Status: NEW → RESOLVED
Last Resolved: 18 days ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.