Closed Bug 966684 Opened 10 years ago Closed 10 years ago

Clicking bookmarks star in menu panel is completely broken (closes panel, doesn't open subview)

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 29

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [Australis:P1])

Attachments

(1 file)

Don't know if this broke because of bug 964887 or because of bug 948213 but it's my fault either way.
We've got a winner.
Blocks: 948213
Status: NEW → ASSIGNED
This is basically because aEvent.originalTarget in maybeAutoHidePanel is the inner button instead of the outer one, and we only check the inner button for the closemenu and/or widget-type attribute, but the bookmarks code sets it on the outer one, which used to work.
This patch makes me sad, but it fixes the issue and I can't think of something much better. We could make the bookmarks widget workaround the issue, but that leaves add-on devs in the cold if they have similar buttons.
Attachment #8369123 - Flags: review?(mconley)
Comment on attachment 8369123 [details] [diff] [review]
check for closemenu and widget type attributes properly in Australis menu panel,

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

I guess it's for the best. :)
Attachment #8369123 - Flags: review?(mconley) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/463bae14bef3
Whiteboard: [Australis:P1] → [Australis:P1][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/463bae14bef3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P1][fixed-in-fx-team] → [Australis:P1]
Target Milestone: --- → Firefox 29
This patch produces a regression in Metro ... 

Normally I can switch from desktop to Metro by pressing the "Windows 8 Touch" icon which I've customized / placed first / top-left in the address-bar line.

With this patch I can't switch any longer, browser console complains about the line
   closemenu = target.getAttribute("closemenu");

Says basically getAttribute is not a function.

Backing out your change locally fixes the issue for me.
Depends on: 966956
QA Contact: cornel.ionce
Verified fixed on Firefox 29 beta 1 - build ID: 20140318013849.
Tested on Windows 7 64bit, Ubuntu 12.04 and Mac OS X 10.9

User Agents:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: