1450657 - Remove ResolveTag usage from nsMenuPopupFrame.

Status: RESOLVED FIXED

(Core :: XUL, task)

Description

[Emilio Cobos Álvarez (:emilio)]

We never extend xul:tooltip, xul:menupopup or xul:popup, so this can go away.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1450657: Remove ResolveTag usage in nsMenuPopupFrame.

We never extend xul:tooltip, xul:menupopup or xul:popup, so this can go away.

Review commit: https://reviewboard.mozilla.org/r/233000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233000/

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8964260 [details]
Bug 1450657: Remove ResolveTag usage in nsMenuPopupFrame.

https://reviewboard.mozilla.org/r/233000/#review238460

https://searchfox.org/comm-central/source/calendar/base/content/calendar-menus.xml#13 does extend xul:menupopup

Please ping the comm-central folks to let them know this is going away.  Looks like this affects the "task-menupopup" tagname; they can presumably replace this with a "menupopup" with some attribute set.  Assuming this isn't dead code, of course; I see no uses of the "task-menupopup" tagname.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Read comment 2. I don't think I'm on a rush to land this, it's just general cleanup. So let me know if comm-central would like me to delay landing this a bit. Though per that comment I don't think it's relevant.

Comment 4

[Brian Grinstead [:bgrins]]

It looks like the bindings that extend task-menupopup (task-priority-menupopup and task-progress-menupopup) do get bound to non-menupopup elements (arrowscrollbox). Does the `extends="xul:menupopup"` also apply to child bindings that extend the xul: tag?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to Brian Grinstead [:bgrins] from comment #4)
> It looks like the bindings that extend task-menupopup
> (task-priority-menupopup and task-progress-menupopup) do get bound to
> non-menupopup elements (arrowscrollbox). Does the `extends="xul:menupopup"`
> also apply to child bindings that extend the xul: tag?

Yes, it looks through all the base binding chain... It's kinda weird that those are bound to arrowscrollboxes, but that indeed may be affected by this change in comm-central.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5)
> (In reply to Brian Grinstead [:bgrins] from comment #4)
> > It looks like the bindings that extend task-menupopup
> > (task-priority-menupopup and task-progress-menupopup) do get bound to
> > non-menupopup elements (arrowscrollbox). Does the `extends="xul:menupopup"`
> > also apply to child bindings that extend the xul: tag?
> 
> Yes, it looks through all the base binding chain... It's kinda weird that
> those are bound to arrowscrollboxes, but that indeed may be affected by this
> change in comm-central.

OK good to know. And yes, it is a bit confusing since that tag won't get the normal arrowscrollbox binding. As :bz pointed out it should be able to be modified to use a menupopup instead - or if it's not important that it gets considered a menupopup then task-menupopup could just remove it's [extends].

Comment 7

[Jorg K (CEST = GMT+2)]

Richard is the expert in bindings. Calendar, being a non-bootstrapped add-on is non-functional right now, so a little hard to test anything. We might just file a follow-up bug to cover it.

Comment 8

[Richard Marti (:Paenglab)]

Calendar is actually broken because of the extension rework. I see only the element defined in CSS and in the XML file but not in other code, so I think it's okay to land now.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> It's kinda weird that those are bound to arrowscrollboxes

Oh, I had missed that the selector there had a combinator.

I don't see how it can  make sense to attach an extends="xul:menupopup" thing to arrowscrollboxes.  But in any case, extends="" does not affect selector matching, and neither arrowscrollbox nor menupopup get boxes by tag name.  The only way to get an nsMenuPopupFrame is to have a -moz-popup display style, which afaict only happens in xul.css for popup/menupopup/panel/tooltip tagnames and in some comm-central stuff for #folderLocationPopup and .autocomplete-result-popup neither of which is in calendar and neither of which applies to these arrowscrollboxes afaict.

So those elements don't have an nsMenuPopupFrame anyway and aren't affected by this patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/479b6858ea6c
Remove ResolveTag usage in nsMenuPopupFrame. r=bz

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/479b6858ea6c