Closed Bug 947586 Opened 7 years ago Closed 7 years ago

Menu panel context menu sometimes only has a single "Add More Items"

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: u428464, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

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

Attachments

(3 files)

STR : 
1. Put the bookmarks widget in the menu panel
2. Leave customization mode
3. Right click on the widget in the menu

Current result : Contextual menu only show "add more items"

ER : Contextual menu shows "add to toolbar", "remove from menu" and "add more items..."

Also note that if you right click the bookmark widget and then another widget they then only show "add more items..." too.
Whiteboard: [Australis:P2]
This widget is a nightmare. I've been able to completely break the customization mode just by randomly using the contextual menu and moving it.
The actual impact here is probably more of a P3 or P4, but I'm going to leave it at P2 until we know what's causing this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch TestSplinter Review
Test that reproduces the issue. When this happens, the context attribute disappears from the buttons in the panel, not sure why though.
This isn't specific to the bookmarks button.

STR:

0) New profile
1) [optional] right-click any icon in the menu panel, confirm that it has the 3 expected "Move to Toolbar" + "Remove from Menu" + "Customize…" items.
2) Enter customize mode
3) right-click any icon in the menu panel shown for customization. It'll have the same items as #1, but "Customize…" is disabled.
4) Exit customize mode
5) right-click any icon in the menu panel. Now there is only a single "Add More Items…" entry.

Note that the single-item "Add More Items…" is the normal and expected context menu when you click empty areas of the menu panel (or the customize/help/quit buttons at the bottom).
Summary: The bookmark widget has wrong contextual menu in menu panel → Menu panel context menu sometimes only has a single "Add More Items"
Attached patch Patch v.1Splinter Review
So... createWrapper() tries to optimize (?) not saving the existing context menu attribute if it's already set to the expected value. I think it's assuming it can safely just nuke this attribute as if "expected" means "default", but that's not true. The panel has a default context menu -- the single-entry "Add More Items…" menu -- and so if we don't restore the |context| attribute for the items in the panel, they will no longer override the panel's default after exiting customization.
Assignee: nobody → dolske
Attachment #8349163 - Flags: review?(jaws)
Whiteboard: [Australis:P2] → [Australis:P3]
Comment on attachment 8349163 [details] [diff] [review]
Patch v.1

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

This patch fails the existing browser_880164_customization_context_menus.js test with "Uncaught exception - Context menu (toolbar-context-menu) did not show within 20 seconds." and there are also some failures with the test attached to this bug but those could be related or fallout from this failure.
Attachment #8349163 - Flags: review?(jaws) → review-
Whiteboard: [Australis:P3] → [Australis:P3][maybe-fix-on-aurora]
Whiteboard: [Australis:P3][maybe-fix-on-aurora] → [Australis:P3][can-fix-on-aurora]
Duplicate of this bug: 963726
Blocks: 945191
Keywords: regression
Ugh, I should have looked at blame to see that 945191 added thia code for a reason (even though it caused this bug).

I tried to instead just a call to ensureButtonContextMenu() when unwrapping items without a cached contextmenu attribute, but that's still broken. I don't understand what's happening now.
Assignee: dolske → nobody
Dolske, at least your comments made it easier for us to figure out how to fix it, so thank you for working on this\! :-) Here's a fix + test which passes on my machine (together with the other menu tests).
Attachment #8369272 - Flags: review?(jaws)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #8369272 - Flags: review?(jaws) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/218d134a0e35
Whiteboard: [Australis:P3][can-fix-on-aurora] → [Australis:P3][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/218d134a0e35
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.