1911486 - dataTransfer is null in content process on dragend

Status: RESOLVED FIXED

(Core :: Widget, defect, P2)

Description

[bobo1239]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:130.0) Gecko/20100101 Firefox/130.0

Steps to reproduce:

• Install the Sidebery extension (https://addons.mozilla.org/en-US/firefox/addon/sidebery/); Alternatively Tree Style Tab also can show the problem
• Create a 2nd tab (so the window stays open)
• From the tab tree bar (added by Sidebery) drag a tab outside the Firefox window

Actual results:

Nothing

Expected results:

A new window should spawn with the tab that was moved out. The tab should not be in the initial window anymore.

Comment 1

[bobo1239]

I've tracked down the regression to the changes introduced by Bug 1893119. The following is my understanding of the issue which I've posted on the original Sidebery issue tracker:

I've bisected the issue to this Firefox change which makes sense since it touches the drag'n drop subsystem. The effect is that in this Sidebery line e.dataTransfer is now null instead of a DataTransfer object. (which alters our control flow) According to the MDN docs this should never happen when the event is dispatched by the browser which is the case here so I'd say this is a Firefox bug. Unfortunately I don't have time to dive more deeply into the Firefox change so somebody else will have to do that...

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[bobo1239]

Looking at the initial post of the Sidebery issue this also happens on Windows 11 so is maybe/probably a platform-independent issue.

Comment 4

[David Parks [:handyman]]

Thanks for the report. I can easily reproduce this with Windows 11. It looks like the rest of DND with tabs, including dragging a tab into the sidebar, is working as expected. The only issue I see is the one reported. The DataTransfer should be the same as for normal in-browser tab dragging, which works, so it's not clear why it isn't but maybe the problem isn't too deep.

Comment 5

[David Parks [:handyman]]

Attachment: Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend r=#win-reviewers!

During EndDragSession and the dispatch of the dragend event in a content process
(e.g. in an extension), accessing the event's dataTransfer didn't work because
the session was already disconnected from the BrowserChild. This broke things
like tab dragging from web-extension-based tabbars. This leaves the drag
session in place until after EndDragSession is done.

Comment 6

[sikander]

e.dataTransfer being null is also affecting this fire-drag extension, making it inoperable.

Comment 7

[Sander Bouwhuis]

Ah man, I too use the fire-drag extension. I'm really bummed out this bug is preventing us from using it.
Can I download v128.x and overwrite the buggy v129 version?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/330ebf99b047
Leave DragSession connected to BrowserChild while sending dragend r=win-reviewers,gstoll

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/330ebf99b047

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:handyman, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox130 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[David Parks [:handyman]]

Comment on attachment 9418000 [details]
Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend r=#win-reviewers!

Beta/Release Uplift Approval Request

• User impact if declined: Some drag-drop operations will not work properly. In particular, extensions that perform DND operations lose some functionality. This is likely true for some sites as well.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change sets the BrowserChild owner of a drag session in a content process earlier than before and clears it later than before. FWIW, this behavior is tested in bug 1912340 (and passes). But that hasn't landed yet.
• String changes made/needed:
• Is Android affected?: No

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9418000 [details]
Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend r=#win-reviewers!

Approved for 130 beta 6, thanks.

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/ae10b9aaa98a

Comment 15

[Donal Meehan [:dmeehan]]

:handyman could you consider adding a release uplift request on this also?
I could include it in the planned Fx129 release if I have one in time.

Comment 16

[David Parks [:handyman]]

Comment on attachment 9418000 [details]
Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend r=#win-reviewers!

Beta/Release Uplift Approval Request

• User impact if declined: Same as beta -- Some drag-drop operations will not work properly. In particular, extensions that perform DND operations lose some functionality. This is likely true for some sites as well.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Also see beta uplift: The change sets the BrowserChild owner of a drag session in a content process earlier than before and clears it later than before.
• String changes made/needed: N/A
• Is Android affected?: No

Comment 17

[Donal Meehan [:dmeehan]]

Comment on attachment 9418000 [details]
Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend r=#win-reviewers!

Approved for 129.0.2

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/bb6e4b7d66e7

Comment 19

[Greg Stoll :gstoll]

Attachment: Bug 1911486: Leave DragSession connected to BrowserChild while sending dragend

During EndDragSession and the dispatch of the dragend event in a content process
(e.g. in an extension), accessing the event's dataTransfer didn't work because
the session was already disconnected from the BrowserChild. This broke things
like tab dragging from web-extension-based tabbars. This leaves the drag
session in place until after EndDragSession is done. It also makes the
reciprocal change to InvokeDragSession.

Original Revision: https://phabricator.services.mozilla.com/D218667

Comment 20

[Greg Stoll :gstoll]

esr128 Uplift Approval Request

• User impact if declined: drag and drop with some extensions may be broken
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: medium
• Explanation of risk level: drag and drop changes, but they've already made it to release
• String changes made/needed: no
• Is Android affected?: no

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/ff01ad25cce5