978084 - Attribute [customizing-movingItem] set but not removed.

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[mcdavis941 (sporadically reading bugmail)]

This is based on looking in DOM Inspector, not from looking deeply at the underlying code:

When you start a drag-and-drop of a movable item while customizing in Australis, attribute [customizing-movingItem] is set on #main-window but isn't removed.  The code makes it seem like it's meant to be removed when the drag-and-drop operation is completed, but the attribute stays set anyway.  I have no idea if this matters or not, but this seems suspicious, so I'm mentioning it FWIW.

This is with Fx29 Aurora.

Comment 1

[:Gijs (he/him)]

Matt, is this something you could look at when looking at bug 977862?

Comment 2

[Matthew N. [:MattN]]

Yep

Comment 3

[Matthew N. [:MattN]]

Attachment: v.1 Ensure dragend gets fired to workaround bug 460801

https://developer.mozilla.org/en-US/docs/Web/Reference/Events/dragend#Notes

Comment 4

[:Gijs (he/him)]

Comment on attachment 8385358 [details] [diff] [review]
v.1 Ensure dragend gets fired to workaround bug 460801

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

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +1353,5 @@
>        } else {
>          // The items in the palette are wrapped, so we need the target node's parent here:
>          this.visiblePalette.insertBefore(draggedItem, aTargetNode.parentNode);
>        }
> +      if (aOriginArea.id !== kPaletteId) {

Please add a comment here about why this is necessary, and why only if the origin /isn't/ the palette.

@@ +1393,5 @@
>        // as a "move". An "add" is only when we move an item from the palette into
>        // an area.
>        let custEventType = aOriginArea.id == kPaletteId ? "add" : "move";
>        BrowserUITelemetry.countCustomizationEvent(custEventType);
> +      this._onDragEnd(aEvent);

Ditto.

Comment 5

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/460737714c56

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/460737714c56

Comment 7

[:Gijs (he/him)]

Comment on attachment 8385358 [details] [diff] [review]
v.1 Ensure dragend gets fired to workaround bug 460801

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: some styles might not be applied consistently, issues with add-ons checking for window attributes
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): very low, has automated test
String or IDL/UUID changes made by this patch: none

Comment 8

[:Gijs (he/him)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/59e8f1a71318