Closed Bug 1371236 Opened 3 years ago Closed 3 years ago

[Photon] Overflow panel shouldn't try to open in customize mode when dragging items over it

Categories

(Firefox :: Toolbars and Customization, defect, P1)

53 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.1 - Jun 26
Tracking Status
firefox56 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-structure])

Attachments

(1 file)

We've made the overflow button visible in customize mode, but now its panel shows up when dragging items on top of it (despite the button being marked disabled, and the panel empty because its contents are in customize mode).

The drag handlers should either be removed in customize mode and then readded, or (probably simpler) check whether the button is disabled before doing anything.

Happily, this doesn't happen with the photon pref disabled.
Flags: qe-verify+
Priority: -- → P3
QA Contact: gwimberly
Iteration: --- → 56.1 - Jun 26
Priority: P3 → P1
Whiteboard: [reserve-photon-structure] → [photon-structure]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8880391 [details]
Bug 1371236 - don't try to open the overflow panel when in customize mode,

https://reviewboard.mozilla.org/r/151760/#review156734

::: browser/components/customizableui/CustomizableUI.jsm:4183
(Diff revision 1)
> -      let overflowableToolbarInstance = this;
> -      this._panel.addEventListener("popupshown", function(aEvent) {
> -        this.addEventListener("dragover", overflowableToolbarInstance);
> +      this._panel.addEventListener("popupshown", (aEvent) => {
> +        this._panel.addEventListener("dragover", this);
> +        this._panel.addEventListener("dragend", this);
> -        this.addEventListener("dragend", overflowableToolbarInstance);

Not strictly necessary, but I noticed this and thought I might as well tidy it up a bit.
Comment on attachment 8880391 [details]
Bug 1371236 - don't try to open the overflow panel when in customize mode,

https://reviewboard.mozilla.org/r/151760/#review157308

Cool, nice and simple. Thanks for the drive-by cleanup.

::: browser/components/customizableui/CustomizableUI.jsm:4183
(Diff revision 1)
>        // case the edit controls are in it.
>        this._panel.addEventListener("popupshowing", () => doc.defaultView.updateEditUIVisibility(), {once: true});
>        this._panel.openPopup(anchor || this._chevron);
>        this._chevron.open = true;
>  
> -      let overflowableToolbarInstance = this;
> +      this._panel.addEventListener("popupshown", (aEvent) => {

nit, drop the parens on aEvent
Attachment #8880391 - Flags: review?(jaws) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5af23c6f9303
don't try to open the overflow panel when in customize mode, r=jaws
https://hg.mozilla.org/mozilla-central/rev/5af23c6f9303
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
I have reproduced this bug with nightly 55.0a1 (2017-06-08)  on ubuntu 16.04(64 Bit).

The bug's fix is now verified on Latest Nightly 56.0a1 .

Build ID 	20170627100221
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170628]
I have reproduce this bug with Nightly 55.0a1 (2017-06-08)) on Windows 8.1 (64 Bit).

This bug's fix is verified on Latest Nightly 56.0a1.

Build ID : 20170627030209
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170628]
As per Comment 7 and Comment 8, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.