Closed Bug 878452 Opened 11 years ago Closed 11 years ago

Dragging an item from the palette to the panel (not to an item in the panel) doesn't work when the panel is empty.

Categories

(Firefox :: Toolbars and Customization, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:M6])

Attachments

(1 file)

STR:
1. Clear the panel by moving everything out of it into the palette.
2. Try to drag something back in.

Expected: works.
Actual: doesn't! :-)

I'll take this and force myself to do an actual dnd thing for testing...
The patches I'm working on for bug 877370 should fix this.
Of course, smaller patches are better. I'd be happy to rebase on top of your work.
Summary: Dragging an item from the palette to the panel (not to an item in the panel) doesn't work. → Dragging an item from the palette to the panel (not to an item in the panel) doesn't work when the panel is empty.
Attached patch Patch with testsSplinter Review
This is a minimal patch which fixes this issue and adds tests. :-)

(based on the latest test framework stuff after the patches from bug 877178 land)
Attachment #757363 - Flags: review?(jaws)
Also, I filed bug 878762 for an issue with the add/move/remove APIs when in customization mode. They are designed to be used when not in customization mode, and when the customization mode calls them it takes care to unwrap the nodes and then wrap them again. This is why the last test in the file (which is the one which doesn't work without the null check in _setDragActive) is doing its emptying of the panel before entering customization mode.

Separately, I am concerned about not seeing any errors in the console from the _setDragActive code... should we wrap the contents of handleEvent in a try catch and report errors ourselves? Or was this only because it was a mochitest and would I have seen things in the browser console?
(In reply to :Gijs Kruitbosch from comment #4)
> Separately, I am concerned about not seeing any errors in the console from
> the _setDragActive code... should we wrap the contents of handleEvent in a
> try catch and report errors ourselves? Or was this only because it was a
> mochitest and would I have seen things in the browser console?

The LOG calls in the ondrag* functions that are called by handleEvent are working. I do think there is an issue with the logging of exceptions being thrown from within handleEvent.

It may be a good idea to do like so:

handleEvent: function(aEvent) {
  try {
    ...
  } catch (ex) {
    LOG(ex);
    throw ex;
  }
},

We should also file a bug to fix the root cause of exceptions not being logged from within event handlers.
Comment on attachment 757363 [details] [diff] [review]
Patch with tests

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

::: browser/components/customizableui/test/browser_878452_drag_to_panel.js
@@ +10,5 @@
> +      let btn = document.getElementById("developer-button");
> +      let panel = document.getElementById(CustomizableUI.AREA_PANEL);
> +      let placements = getAreaWidgetIds(CustomizableUI.AREA_PANEL);
> +
> +      let lastButtonIndex = placements.length -1;

nit, spaces on both sides of '-'.
Attachment #757363 - Flags: review?(jaws) → review+
Pushed: https://hg.mozilla.org/projects/ux/rev/e878799f7d5a
Status: NEW → ASSIGNED
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
Blocks: 879098
Backed out (https://hg.mozilla.org/projects/ux/rev/65fde4f8d09b) because of privatebrowsing test failures, see bug 879098.
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Pushed again with test fixes: https://hg.mozilla.org/projects/ux/rev/1a274e647119
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
https://hg.mozilla.org/mozilla-central/rev/1a274e647119
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
Is it related to bug 1005677?
(In reply to Alexandre Magno from comment #11)
> Is it related to bug 1005677?

No.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: