Closed Bug 941109 Opened 7 years ago Closed 7 years ago

Australis: Overflow&menu panels should respect closemenu xul attribute in children

Categories

(Firefox :: Menus, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: nohamelin, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: addon-compat, Whiteboard: [Australis:P3])

Attachments

(1 file, 1 obsolete file)

My Simple Locale Switcher extension has the following xul code for a toolbar button:

<toolbarbutton type="menu">
  <menupopup>
    <menuitem type="checkbox" closemenu="none"/>
    ...

If this button is placed inside the new menu panel or the overflow panel, toggling the checkbox keep the button's popup open *but* the parent panel is closed. See:

https://developer.mozilla.org/en-US/docs/XUL/Attribute/closemenu
See Also: → 940693
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Australis:P3]
From comment #5 of bug 940693, noautoclose="true" seems to work as I expected of closemenu="none". It seems that closemenu is for panels as closemenu is for menus. Or maybe the old was simply missed? It seems be hardly used.
(In reply to Carlos [:nohamelin] from comment #1)
> From comment #5 of bug 940693, noautoclose="true" seems to work as I
> expected of closemenu="none". It seems that closemenu is for panels as
> closemenu is for menus. Or maybe the old was simply missed? It seems be
> hardly used.

Yeah, I don't think we realized there was an existing attribute to use for this. As we're still on Nightly and probably won't go to Aurora in 3 weeks already, we should evaluate if we can switch to the existing attribute or if that somehow doesn't make sense. Dão, what do you think?
Flags: needinfo?(dao)
We just talked to Dao IRL, and we're going to attempt to support the closemenu attribute in the panel for toolbarbuttons as well as menuitems of the panel's children. This will supplant the noautoclose attribute that we're currently using.
Flags: needinfo?(dao)
Whiteboard: [Australis:P3] → [Australis:P3][investigate-fix-on-aurora]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Keywords: addon-compat
Whiteboard: [Australis:P3][investigate-fix-on-aurora] → [Australis:P3]
I think my editor's search/replace works. The tests agree. Mike, do you, too? :-)
Attachment #8362607 - Flags: review?(mconley)
Comment on attachment 8362607 [details] [diff] [review]
use closemenu instead of noautoclose attribute in Australis menupanel,

Review of attachment 8362607 [details] [diff] [review]:
-----------------------------------------------------------------

DXR confirms that these are all of the instances of noautoclose being used.

This looks good to me - though we might want to think about supporting closemenu="single", and what that means for things like items in subviews.

::: browser/components/customizableui/content/panelUI.js
@@ +329,5 @@
>     * This function can be used as a command event listener for subviews
>     * so that the panel knows if and when to close itself.
>     */
>    onCommandHandler: function(aEvent) {
> +    if (aEvent.originalTarget.getAttribute("closemenu") != "none") {

What about if closemenu is "single" in this case? Or would you rather support "single" in a follow-up?
Attachment #8362607 - Flags: review?(mconley) → review+
This adds support for closemenu == 'single' to the command handler. Thinking about it though, I don't really see a usecase for it... but hey, it wasn't hard to do.
Attachment #8362695 - Flags: review?(mconley)
Attachment #8362607 - Attachment is obsolete: true
Comment on attachment 8362695 [details] [diff] [review]
use closemenu instead of noautoclose attribute in Australis menupanel,

Review of attachment 8362695 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me. Tests would be nice, but I'm not going to block on that.
Attachment #8362695 - Flags: review?(mconley) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/868c2aeb6e55

There are some tests in the patch on bug 940307.
Whiteboard: [Australis:P3] → [Australis:P3][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/868c2aeb6e55
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 29
Depends on: 962855
No longer depends on: 962855
You need to log in before you can comment on or make changes to this bug.