Make it impossible to enter the native drag and drop loop on Windows in test automation

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P5
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

unspecified
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

We have some mochitests that simulate drag and drop operations. This can be hazardous, at least on Windows, because if the widget code actually gets the directive to start an actual drag and drop operation, we'll enter this native loop here:

https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/widget/windows/nsDragService.cpp#294

which puts us into this really weird state where Gecko events won't be processed properly until the drag and drop operation exits (which seems to only be possible via actual user input, because script doesn't appear to run when we're in this native loop).

Most tests get around this by using the EventUtils.synthesizeDrag* functions, which call stopPropagation() and preventDefault() on the drag events, which prevents the widget layer from entering the loop.

There are, however, other tests, that don't do this. dom/events/test/test_bug1264380.html is one of those tests.

Normally, that wouldn't be a problem, since in our CI on Windows, the mouse is positioned (I believe) in the very top-left of the screen, which up until now had been considered "outside of the window" to widget code, because that area is a moz-window-dragging region, which hittests to HT_CAPTION, which we were treating as "outside of the client area, and so therefore outside of the window". This meant that the native drag and drop loop would never be entered, because the widget code would go, "Oh, the mouse is outside of the browser window, so this doesn't make sense to start and drag and drop operation inside of it".

With bug 1534389, I'm making those moz-window-dragging regions behave as if they're still inside of the client area so that we can get things like :hover effects on them. This meant that in CI, test_bug1264380.html would enter the ::DoDragDrop native event loop, and the test (or the next test - there's a bit of a delay) would get stuck and eventually time out.

I propose we do the following:

  1. Make test_bug1264380.html use the EventUtils.synthesizeDragStart helper to avoid the native drag and drop loop from being entered, and

  2. Make it so that attempting to invoke the native drag and drop loop in automation causes us to crash.

Updated

3 months ago
Priority: -- → P5
Assignee

Updated

3 months ago
Depends on: 1481666
Assignee

Updated

3 months ago
Assignee: nobody → mconley
Assignee

Comment 2

3 months ago

Hi Neil,

Do you think you'll get a chance to review the new revision of my patch today? I'm hoping to land it this week. Let me know if you're too busy and I'll redirect it.

Flags: needinfo?(enndeakin)

Comment 3

3 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba0820e380a3
Make it impossible to enter the native drag-drop loop in test automation. r=NeilDeakin
Assignee

Comment 4

3 months ago

Thanks, Neil!

Flags: needinfo?(enndeakin)

Comment 5

3 months ago
Backout by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/62d21e6df277
Backed out changeset ba0820e380a3 for Mochitest failures in layout/generic/test/test_bug496275.html. CLOSED TREE

Comment 6

3 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5dae6d1b597a
Make it impossible to enter the native drag-drop loop in test automation. r=NeilDeakin

Comment 7

3 months ago
bugherder
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Comment 9

3 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c523c4658e98
Make it impossible to enter the native drag-drop loop in test automation. r=NeilDeakin

Comment 10

3 months ago
bugherder
Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee

Updated

2 months ago
Flags: needinfo?(mconley)
You need to log in before you can comment on or make changes to this bug.