896596 - _onDragStart hack in CustomizeMode causes placeholders to get inserted after exiting customization mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

simulateItemDrag calls both synthesizeDragStart, and synthesizeDrop - but I believe synthesizeDrop fires its own dragstart event. Two consecutive dragstart events without a drop is unexpected, and results in weird behaviour when dragging items around in customization mode. Might as well just remove the unnecessary synthesizeDragStart.

Setting to P1 because it blocks another P1.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1 (rs=Gijs over IRC)

Comment 2

[Mike Conley (:mconley) (:⚙️)]

So, on closer examination, I'm not sure if my original assessment is correct - synthesizeDrop seems to capture the dragstart event that it generates, and then stops it from propagating...

At least, it's *supposed* to be doing that.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 779317 [details] [diff] [review]
Patch v1 (rs=Gijs over IRC)

Revoking r+ until we understand what's happening here.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

Yeah, my original assessment was way off.

What's going on here is that we have a hack in _onDragStart that uses a setTimeout of 0 to insert the placeholders into the panel. As you might expect, our automated tests sometimes exit before that setTimeout can fire - and that means that the placeholders get injected *after* we've exited customization mode. As Gijs would say, this is sad-making.

So, assuming we have to keep this hack (and I don't want to invest too much time trying to make the hack unnecessary), I've added a conditional to just ensure that we're actually customizing and we're not mid-transition.

Comment 5

[:Gijs (he/him)]

Comment on attachment 779374 [details] [diff] [review]
Patch v2

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

LGTM!

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Thanks Gijs - landed on UX as https://hg.mozilla.org/projects/ux/rev/d7fd9dcb68a8

Comment 7

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/d7fd9dcb68a8