Closed Bug 978084 Opened 6 years ago Closed 6 years ago

Attribute [customizing-movingItem] set but not removed.

Categories

(Firefox :: Toolbars and Customization, defect)

29 Branch
x86_64
Windows Vista
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: mcdavis941.bugs, Assigned: MattN)

References

Details

(Whiteboard: [Australis:P3-])

Attachments

(1 file)

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.
Matt, is this something you could look at when looking at bug 977862?
Flags: needinfo?(MattN+bmo)
Whiteboard: [Australis:P3-]
Yep
Assignee: nobody → MattN+bmo
Blocks: 963576
No longer blocks: australis-cust
Status: NEW → ASSIGNED
Flags: needinfo?(MattN+bmo)
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.
Attachment #8385358 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/460737714c56
Flags: in-testsuite+
Whiteboard: [Australis:P3-] → [Australis:P3-][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/460737714c56
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3-][fixed-in-fx-team] → [Australis:P3-]
Target Milestone: --- → Firefox 30
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
Attachment #8385358 - Flags: approval-mozilla-aurora?
Attachment #8385358 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.