1598915 - Can't drag a tab into a new window above Visual Studio Code

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P3)

Description

[Max Power]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

I have two monitors, A and B. Maximised FireFox was on A, Maximised VS Code was on B. I tried drag a FireFox tab to B.

Actual results:

Nothing happens except when the mouse-release point is on the menu bar (or title bar) of VS Code. That is, if the release point is on the editor or the left pane, nothing happens.

I have heard that VS Code is based on Chromium, so maybe FireFox thought that I am dragging the tab to Chrome? I have tested the same thing with Chrome, but Chrome moved the tab to above VS Code without any problem.

Expected results:

The tab should be moved to the mouse-release point.

Comment 1

[Marcela (Please NI request to Brindusa Tot)]

Hi Max!
As far as I could see, when Firefox is maximized on both monitors, tabs can be dragged from one monitor to another by placing the mouse pointer in the address bar
Do you also want them to be able to drag tabs from any point in the window?

Comment 2

[Max Power]

(In reply to Marcela from comment #1)

No, not two windows of FireFox on each monitors. What I meant was that FireFox was on one monitor, and Visual Studio Code was on the other monitor. VS Code was maximised on that monitor, so when I tried to move a tab of FireFox on the monitor, the mouse happened to be on VS Code when I was releasing the mouse to finish dragging.

Those "maximise" part was just a detailed expiation for the situation. The gist is that when dragging a FireFox tab, if the position where you release the mouse point happens to be on VS Code (other than its menu bar), the drag has no effect.

Comment 3

[Marcela (Please NI request to Brindusa Tot)]

I don´t have VS code installed so I'm not able to reproduce this issue.
Does this happen with VS code and with any other web page?
Could you please send us a screen recorder with this behavior to see the problem?

Comment 4

[Max Power]

Attachment: screen.mp4

(In reply to Marcela from comment #3)

VS Code is a free text editor from Microsoft, so you can download it from https://code.visualstudio.com/.

Anyways, here is the video recording of the symptom. FireFox and VS Code are in their default settings after installation. Notice that I can drag a tab and drop it above the Notepad. Now, when I try to do the same thing above VS Code, nothing happens. I can drag a tab, however, and drop it above the menu bar of VS Code.

Comment 5

[Marcela (Please NI request to Brindusa Tot)]

I was able to reproduce this issue on latest Nightly Version 73.0a1 Build ID 20191205215330 using Windows 10. I also changed Product and Component

Comment 6

[Max Power]

This happens with Blender (version 2.81a), too.

Comment 7

[BugBot [:suhaib / :marco]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Max Power]

FireFox 84.0.2. If I drag a tab and drop it over Android Studio (Android Studio is happened to be the maximised window on that monitor, so it is natural to drop it over the Android Studio window, in such a case), the tab becomes a new window beneath the Android Studio window, for some reason.

And the old problems with VS Code, Blender still exist. How come this issue is not more popular among other users? Android Studio, VS Code, Blender are all popular applications.

Comment 9

[Daniel Green]

I just started using VSCode and have noticed this happening to me as well. It's almost like Firefox is seeing VSCode as a drag-and-drop target for the tab, so it doesn't give the tab its own window like I expect (but it doesn't do anything to VSCode, either.)

This problem also exists when dragging a tab and releasing it over GitHub Desktop for Windows, which makes me think maybe these are web apps masquerading as desktop apps and Firefox treats a web browser drop target differently.

Suffice to say, I have no idea why this is happening but it is very inconvenient to be unable to create a new window from a tab unless I minimize my text editor.

I am on Firefox Developer 91.0b1, Windows 10.

Comment 10

[Cr8zy]

I Second the Inconvenience of this Bug.
I've been accepting this behaviour for the past year, thinking some other program might be causing it. I am now fed up. With respect, please fix this bug.

Comment 11

[VVV]

4 years has passed and we still have this issue. This time with Discord for me.

Comment 12

[Ray Kraesig [:rkraesig]]

(In reply to Daniel Green from comment #9)

This problem also exists when dragging a tab and releasing it over GitHub Desktop for Windows, which makes me think maybe these are web apps masquerading as desktop apps and Firefox treats a web browser drop target differently.

Indeed they are, at least from a certain perspective: Visual Studio Code and GitHub Desktop are both Electron apps, and Electron is (very approximately) a moderately-locked-down variant of Google Chrome that automatically loads a local webpage. I assume Discord is as well.

Blender, however, is decidedly not. Conversely, VS Code's top bar is as much Electron as the rest of it, but doesn't exhibit this behavior. It doesn't even happen on every drop-target: WordPad accepts a dropped-in URL, but not a Firefox tab.

Under the hood, what's happening here is this: Firefox, acting as a drag-and-drop source, offers the tab as an IDataObject; and VS Code, acting as a drag-and-drop target, takes a look at the proffered data-object and responds "sure, I can take that data; I'll make a copy of it" (that is, DROPEFFECT_COPY + S_OK). Firefox sees this and takes no further action, trusting that the receiving application will make it visible in its own way. This trust is completely misplaced, as VS Code proceeds to do nothing of the sort.

If you drop the tab onto the desktop, or onto an application which says "yup, that's some data alright, but it's not for me" (DROPEFFECT_NONE + S_OK), or even onto an application that says "I thought I could handle that, but apparently not" (DROPEFFECT_COPY + E_FAIL), Firefox will go ahead and tear off the tab. It's only when an application responds affirmatively that the tab stays in place.

It's possible that we're doing something wacky in our IDataObject implementation that's confusing other apps -- the relevant code is mostly 15+ years old, and definitely under-documented and under-tested, so I'll need to do some additional probing to be sure -- but it doesn't look like it.

...

Alas, we work with the application ecosystem we have, not necessarily the one that we want. Even if this is objectively a bug in Chromium/CEF, the fact that Blender and IntelliJ independently exhibit the same behavior means it's common enough in the Windows ecosystem that we'll need to mitigate it somehow. In this case, we can probably do that just by overriding Firefox's tab-drop behavior -- there aren't too many applications that should be accepting Firefox tabs, after all.

Comment 13

[Daniel Green]

Great and appreciated reply, Ray. Sounds very reasonable, particularly the part where there aren't many applications that should be accepting Firefox tabs. I don't know if an allowlist is the way to go there, but if the list is small enough...

Comment 15

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1598915 - [1/2] Drive-by cleanup: replace manual pointer-fiddling with RefPtr r?#win-reviewers

nsDataObj::mTransferable's lifecycle and ownership semantics are
trivially that of RefPtr. Use it.

(We do not presently extend this to m_enumFE, below; according to
comments in IEnumFE.h, that one's less trivial.)

No functional changes.

Comment 16

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1598915 - [2/2] Ignore external drag-and-drop responses for Fx tabs r?handyman!

When dragging and dropping a Firefox tab onto another application,
assume the tab-drop is not handled, even if the application claims that
it was.

Comment 17

[Ray Kraesig [:rkraesig]]

(In reply to Daniel Green from comment #13)

Great and appreciated reply, Ray. Sounds very reasonable, particularly the part where there aren't many applications that should be accepting Firefox tabs. I don't know if an allowlist is the way to go there, but if the list is small enough...

For the record: we can't actually do an allowlist in the sense you mean, because we don't know what program is on the other end of the drag-and-drop: we only know if it's being dropped onto us (the same process) or someone else (any other process).

Fortunately, this is exactly the distinction we need to draw, since (as far as I'm aware) the set of applications that actually handle having a Firefox tab dropped on them is {"Mozilla Firefox"} — and even we only allow same-process, same-context tab drops.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/415be9b49528
[1/2] Drive-by cleanup: replace manual pointer-fiddling with RefPtr  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/340e2d1b2d23
[2/2] Ignore external drag-and-drop responses for Fx tabs  r=handyman,win-reviewers

Comment 19

[tszentpeteri]

Backed out for causing build bustages on nsDragService.cpp.

• Backout link
• Push with failures
• Failure Log:

[task 2024-07-16T21:39:27.021Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/widget/windows'
[task 2024-07-16T21:39:27.025Z] 21:39:27     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang-cl -fms-compatibility-version=19.39 -std:c++17 -m32 -Xclang -ivfsoverlay -Xclang /builds/worker/fetches/vs/overlay.yaml -FoUnified_cpp_widget_windows2.obj -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -guard:cf -DNDEBUG=1 -DTRIMMED=1 -DUNICODE -D_UNICODE -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -D_SECURE_ATL -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DWIN32 -D_WIN32 -D_WINDOWS -DWIN32_LEAN_AND_MEAN -DMOZ_UNICODE '-DMOZ_APP_NAME="firefox"' '-DMOZ_TOAST_APP_NAME="Firefox"' -DWINAPI_NO_BUNDLED_LIBRARIES -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DMOZ_SUPPORT_LEAKCHECKING -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/widget/windows -I/builds/worker/workspace/obj-build/widget/windows -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/gfx/cairo/cairo/src -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/toolkit/components/jsoncpp/include -I/builds/worker/checkouts/gecko/toolkit/xre -I/builds/worker/checkouts/gecko/widget -I/builds/worker/checkouts/gecko/widget/headless -I/builds/worker/checkouts/gecko/xpcom/base -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -MD -DMOZILLA_CLIENT -FI /builds/worker/workspace/obj-build/mozilla-config.h -GR- -fcrash-diagnostics-dir=/builds/worker/artifacts -TP -Zc:sizedDealloc- -D_HAS_EXCEPTIONS=0 -Gy -Zc:inline -arch:SSE2 -Gw -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -Z7 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -Werror -W3 -Wbitfield-enum-conversion -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wenum-compare-conditional -Wenum-float-conversion -Wvolatile -Wno-deprecated-anon-enum-enum-conversion -Wno-deprecated-enum-enum-conversion -Wno-deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-microsoft-exception-spec -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -Wno-psabi -Wthread-safety -Wno-error=builtin-macro-redefined -Wno-vla-cxx-extension -Wno-unknown-warning-option -fno-strict-aliasing -Xclang -ffp-contract=off  -Xclang -MP -Xclang -dependency-file -Xclang .deps/Unified_cpp_widget_windows2.obj.pp -Xclang -MT -Xclang Unified_cpp_widget_windows2.obj   Unified_cpp_widget_windows2.cpp
[task 2024-07-16T21:39:27.025Z] 21:39:27     INFO -  In file included from Unified_cpp_widget_windows2.cpp:38:
[task 2024-07-16T21:39:27.025Z] 21:39:27    ERROR -  /builds/worker/checkouts/gecko/widget/windows/nsDragService.cpp(288,33): error: no member named 'widget_windows_allow_external_tab_drag' in namespace 'mozilla::StaticPrefs'
[task 2024-07-16T21:39:27.026Z] 21:39:27     INFO -    288 |       if (mozilla::StaticPrefs::widget_windows_allow_external_tab_drag()) {
[task 2024-07-16T21:39:27.026Z] 21:39:27     INFO -        |           ~~~~~~~~~~~~~~~~~~~~~~^
[task 2024-07-16T21:39:27.026Z] 21:39:27     INFO -  1 error generated.
[task 2024-07-16T21:39:27.026Z] 21:39:27    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:674: Unified_cpp_widget_windows2.obj] Error 1
[task 2024-07-16T21:39:27.027Z] 21:39:27     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/widget/windows'
[task 2024-07-16T21:39:27.032Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/serviceworkers/test/gtest'
[task 2024-07-16T21:39:27.033Z] 21:39:27     INFO -  mkdir -p '.deps/'
[task 2024-07-16T21:39:27.033Z] 21:39:27     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/serviceworkers/test/gtest'
[task 2024-07-16T21:39:27.039Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/serviceworkers/test/gtest'
[task 2024-07-16T21:39:27.039Z] 21:39:27     INFO -  dom/serviceworkers/test/gtest/Unified_cpp_test_gtest0.obj
[task 2024-07-16T21:39:27.040Z] 21:39:27     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/serviceworkers/test/gtest'
[task 2024-07-16T21:39:27.184Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/security/featurepolicy/test/gtest'
[task 2024-07-16T21:39:27.188Z] 21:39:27     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang-cl -fms-compatibility-version=19.39 -std:c++17 -m32 -Xclang -ivfsoverlay -Xclang /builds/worker/fetches/vs/overlay.yaml -FoUnified_cpp_test_gtest0.obj -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -guard:cf -DNDEBUG=1 -DTRIMMED=1 -DUNICODE -D_UNICODE -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -D_SECURE_ATL -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DWIN32 -D_WIN32 -D_WINDOWS -DWIN32_LEAN_AND_MEAN -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DMOZ_SUPPORT_LEAKCHECKING -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/dom/security/featurepolicy/test/gtest -I/builds/worker/workspace/obj-build/dom/security/featurepolicy/test/gtest -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -MD -DMOZILLA_CLIENT -FI /builds/worker/workspace/obj-build/mozilla-config.h -GR- -fcrash-diagnostics-dir=/builds/worker/artifacts -TP -Zc:sizedDealloc- -D_HAS_EXCEPTIONS=0 -Gy -Zc:inline -arch:SSE2 -Gw -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -Z7 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -Werror -W3 -Wbitfield-enum-conversion -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wenum-compare-conditional -Wenum-float-conversion -Wvolatile -Wno-deprecated-anon-enum-enum-conversion -Wno-deprecated-enum-enum-conversion -Wno-deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-microsoft-exception-spec -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -Wno-psabi -Wthread-safety -Wno-error=builtin-macro-redefined -Wno-vla-cxx-extension -Wno-unknown-warning-option -fno-strict-aliasing -Xclang -ffp-contract=off  -Xclang -MP -Xclang -dependency-file -Xclang .deps/Unified_cpp_test_gtest0.obj.pp -Xclang -MT -Xclang Unified_cpp_test_gtest0.obj   Unified_cpp_test_gtest0.cpp
[task 2024-07-16T21:39:27.188Z] 21:39:27     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/security/featurepolicy/test/gtest'
[task 2024-07-16T21:39:27.188Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/security/featurepolicy/test/gtest'
[task 2024-07-16T21:39:27.188Z] 21:39:27     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/security/featurepolicy/test/gtest'
[task 2024-07-16T21:39:27.204Z] 21:39:27     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/editor/txmgr/tests'

Comment 20

[Ray Kraesig [:rkraesig]]

The necessary #include statement was omitted from the patchset. Retrying (after local reconfirmation).

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25b458949cb6
[1/2] Drive-by cleanup: replace manual pointer-fiddling with RefPtr  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/f55b951c694d
[2/2] Ignore external drag-and-drop responses for Fx tabs  r=handyman,win-reviewers

Comment 22

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/25b458949cb6
https://hg.mozilla.org/mozilla-central/rev/f55b951c694d

Comment 25

[Ray Kraesig [:rkraesig]]

This was not quite fixed in Fx130, as unfortunately I seem to have reversed the sense of the test for the relevant pref — it should be effective by default.

Comment 26

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1598915 - [3/2] Fix sense of preference-test r?#win-reviewers!

The sense of the test-condition here was reversed: as the preference's
name states, this should run when the preference is false (as it is by
default).

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49904ac2304f
[3/2] Fix sense of preference-test  r=win-reviewers,handyman

Comment 28

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/49904ac2304f

Comment 29

[BugBot [:suhaib / :marco]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 30

[BugBot [:suhaib / :marco]]

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

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

For more information, please visit BugBot documentation.