Moving an icon around removes custom context menu

VERIFIED FIXED in Firefox 29

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: gaubugzilla, Assigned: Gijs)

Tracking

(Blocks 2 bugs, {regression})

Trunk
Firefox 30
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29 verified, firefox30 verified)

Details

(Whiteboard: [Australis:P2])

Attachments

(1 attachment)

I noticed that toolbar customization will remove the "context" attribute from the Adblock Plus button under some circumstances. Steps to reproduce:

1. Install Adblock Plus.
2. Right-click the Adblock Plus icon - note the custom menu showing up.
3. Start toolbar customization.
4. Move the Adblock Plus icon from the toolbar into the overflow panel, then back.
5. Finish toolbar customization.
6. Right-click the Adblock Plus icon.

Expected results:

Custom Adblock Plus menu opens.

Actual results:

Default menu opens, the context attribute originally present on the button has been removed and not restored (tested in the 2014-03-06 Firefox nightly on OS X). Adding regression keyword because I am pretty certain that this used to work correctly immediately after the Australis merge.
(In reply to Wladimir Palant from comment #0)
> 4. Move the Adblock Plus icon from the toolbar into the overflow panel, then
> back.

You mean the main menu panel, right? The overflow panel is the thing that shows up when you make the window narrow and then click the chevron that shows all the items that didn't fit...
Whiteboard: [Australis:P2]
Yes, I guess the main menu panel is what I meant.
I've written an automated test for this already, just need to figure out why it's breaking. :-\
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
This took a bit in order not to break stuff... but I believe this is sane, and it passes all the tests (including the new one I added for this particular problem). The root cause was the sizing checks already doing wrapping/unwrapping, which puts the context menu back in its 'normal' attribute, which then means it's not in the wrappedcontext attribute, which means stuff breaks.
Attachment #8389115 - Flags: review?(mconley)
Comment on attachment 8389115 [details] [diff] [review]
moving icon around removes custom context menu,

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

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +1752,5 @@
>  
>      if (targetArea != currentArea) {
>        this.unwrapToolbarItem(aDraggedItem.parentNode);
>        // Put the item back into its previous position.
> +      currentParent.insertBefore(aDraggedItem, currentSibling);

thank you for tidying this up
Attachment #8389115 - Flags: review?(mconley) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/d6869ad04b24
Whiteboard: [Australis:P2] → [Australis:P2][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d6869ad04b24
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P2][fixed-in-fx-team] → [Australis:P2]
Target Milestone: --- → Firefox 30
Comment on attachment 8389115 [details] [diff] [review]
moving icon around removes custom context menu,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: icons' custom menus go missing when moving them in customize mode
Testing completed (on m-c, etc.): on m-c, has automated test
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8389115 - Flags: approval-mozilla-aurora?
Attachment #8389115 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: cornel.ionce
Reproduced on the 03/11 Nightly with Adblock Plus and Firebug. Verified as fixed on Firefox 29.0b3 and the latest 30.0a2. All the testing was performed on Mac OS X 10.9.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.