1362065 - Allow touch-based drag/drop of tabs by "touchstart, touchmove, touchend" gesture (in addition to/as opposed to the current double-tap-drag gesture)

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

[Stephen Horlander [:shorlander]]

Hardware: Surface Pro 2
Windows version: Windows 10 — 1703 "Creators Update"


On Nightly drag and drop appears to be broken.

This affects dragging tabs, Customization Mode, dragging tiles on New Tab and content HTML based drag and drop.

The release version of Firefox (53) works fine for me.

Comment 1

[Stephen Horlander [:shorlander]]

Jim is this a known issue?

Comment 2

[Jim Mathies [:jimm]]

Not to me, first report I've seen. Tracy, please try to reproduce and if you can, find a regression range please.

Comment 3

[Stephen Horlander [:shorlander]]

Blake and Johann reported the same issue on different devices.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

If it helps, it fails for me on DevEdition-54, Release-52, and Release-53…
And I'm on Windows 10 Pro, 64-bit, version 1703, build 15063.138.

Comment 5

[Jim Mathies [:jimm]]

Did we turn on a new touch or pointer event feature maybe?

Comment 6

[Tracy Walker [:tracy]]

From comment 4, I'd hazard to guess this is not a regression in our code.  We wouldn't be pushing new feature code to Release-52 or Release-53

I am unable to reproduce this with latest Nightly on Surface Pro Win 10 version 1607.  The device is currently downloading a system update.  I'll see if it updates to version 1703 and if D-n-D then breaks.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

https://www.thurrott.com/windows/windows-10/83780/complete-guide-windows-10-version-1703 suggests that there were a bunch of Precision Touchpad changes.  (I wonder if it's a precision touchpad problem?)

Comment 8

[Tracy Walker [:tracy]]

After update of the device, it is still on version 1607.  I have an older Surface, perhaps that's why it doesn't update to 1703? 

Anyway,  still unable to reproduce any Drag and Drop issues.

Comment 9

[Stephen Horlander [:shorlander]]

(In reply to Tracy Walker [:tracy] from comment #8)
> After update of the device, it is still on version 1607.  I have an older
> Surface, perhaps that's why it doesn't update to 1703? 
> 
> Anyway,  still unable to reproduce any Drag and Drop issues.

They are rolling it out slowly. You can manually do the update: https://blogs.windows.com/windowsexperience/2017/04/11/how-to-get-the-windows-10-creators-update/

Comment 10

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Just adding that it's also happening on my Surface Pro 4 in Nightly 2017-05-08, but not in release.

Comment 11

[Jim Mathies [:jimm]]

I'm seeing this on a surface pro 4 tablet as well with release. One difference I can think of, we have pointer events enabled on nightly currently. The pref is 'dom.w3c_pointer_events.enabled'.

Comment 12

[Jim Mathies [:jimm]]

Are you dragging using touch or via the mouse? I'm not sure what's supposed to work if it's touch, can you provide a simple set of STR to repo?

Comment 13

[Stephen Horlander [:shorlander]]

(In reply to Jim Mathies [:jimm] from comment #12)
> Are you dragging using touch or via the mouse? I'm not sure what's supposed
> to work if it's touch, can you provide a simple set of STR to repo?

Dragging with Touch

Comment 14

[Stephen Horlander [:shorlander]]

An example STR:

- Try to touch and swipe to drag a tab to reorder
- Touch and swipe appears to do nothing

Expected result:
- Tab is detached and you can start reordering

Apply same STR to tiles, customize mode, etc.

Comment 15

[Tracy Walker [:tracy]]

ah, touch screen D-n-D.  Yes, I can confirm on latest Nightly (64 bit) this is broken.  However, as reported, it works on Release 53 (64 bit).

Jim, I also tried with dom.w3c_pointer_events.enabled set to false.  Doing so had no affect.  I also tried detaching the keyboard and switching to tablet mode.  No change, still no touch screen D-n-D

Comment 16

[Tracy Walker [:tracy]]

Trying to find a regression range here.  But tested back to Nightly 53a1 of Jan 1 of this year.  It is broken there too.  Which doesn't make sense, since touch screen D-n-D is working on release of 53.  

Is there some sort of setting or pref on Nightly which gets flipped on/off for Release that could cause this?

Comment 17

[Jim Mathies [:jimm]]

(In reply to Tracy Walker [:tracy] from comment #16)
> Trying to find a regression range here.  But tested back to Nightly 53a1 of
> Jan 1 of this year.  It is broken there too.  Which doesn't make sense,
> since touch screen D-n-D is working on release of 53.  
> 
> Is there some sort of setting or pref on Nightly which gets flipped on/off
> for Release that could cause this?

One of the conclusions here is that this is tied to Win10 creator's update. That might be incorrect. Do you have any touch hardware that hasn't updated yet?

windows 10 anniversary update - 1607 
creators update - 1703

Comment 18

[Jim Mathies [:jimm]]

We shipped touch events in 52, so doesn't look like that plays into this bug.

Bug 1244402 - Let touch events on windows ride the trains
https://bugzilla.mozilla.org/show_bug.cgi?id=1244402#c8
https://developer.mozilla.org/en-US/docs/Web/API/Touch_events

Comment 19

[Tracy Walker [:tracy]]

Yes, all the testing I have done on this has been with Surface Pro at 1607.  At this point, I don't think the system update is related.  

Is there a list of settings we currently flip off for Release.

Comment 20

[Jim Mathies [:jimm]]

(In reply to Tracy Walker [:tracy] from comment #19)
> Yes, all the testing I have done on this has been with Surface Pro at 1607. 
> At this point, I don't think the system update is related.  
> 
> Is there a list of settings we currently flip off for Release.

There's as lot of variation there. Don't think we track the differences.

Testing on a device I had laying around - 

Win10 Creators works with Firefox 48.0b10, 54.0b6

Comment 21

[Stephen Horlander [:shorlander]]

Maybe relevant: Tested Nightly with e10s disabled and Touch D-n-D worked.

Comment 22

[Tracy Walker [:tracy]]

Dug deeper for regression range and found this has regressed in two different ways.

First)  finger touch D-n-D broke with Nightly changeset from 20170119 (works) and 20170120 (broken): 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90d8afaddf9150853b0b68b35b30c1e54a8683e7&tochange=99a239e1866a57f987b08dad796528e4ea30e622  perhaps caused by: Neil Deakin — Bug 1301673

Second)  touch pen D-n-D broke more recently with Nightly changeset from 20170301 (works) and 20170302 (broken): 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=34c6c2f302e7b48e3ad2cec575cbd34d423a9d32&tochange=e91de6fb2b3dce9c932428265b0fdb630ea470d7


Should the later be split off into a separate bug?

Comment 23

[Tracy Walker [:tracy]]

(In reply to Tracy Walker [:tracy] from comment #22)

> First)  finger touch D-n-D broke with Nightly changeset from 20170119
> (works) and 20170120 (broken): 

Dates are wrong in the above post *sigh*  it should read:
Nightly changeset from 20161019 (works) and 20161020 (broken):

The changelog linked above is correct.

Also I forgot to mention that the touch pen regression is larger than just D-n-D. Touch pen support is entirely broken as of Nightly build of 20170302.  Which is bug 1351833

Comment 24

[Stephen Horlander [:shorlander]]

Any updates on this? Makes Firefox really broken on touch devices.

Comment 25

[Jim Mathies [:jimm]]

[Tracking Requested - why for this release]:
crap, this bug is rolling out in 55.

Comment 26

[Kartikaya Gupta (email:[e-mail])]

:shorlander, what gesture are you using for drag and drop? Are you doing a double-tap-drag, or just a simple "finger down, drag"?

Comment 27

[Stephen Horlander [:shorlander]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #26)
> :shorlander, what gesture are you using for drag and drop? Are you doing a
> double-tap-drag, or just a simple "finger down, drag"?

Finger down, drag.

Comment 28

[Kartikaya Gupta (email:[e-mail])]

Try the other one. It works for me on Nightly.

Comment 29

[Stephen Horlander [:shorlander]]

(In reply to Jim Mathies [:jimm] from comment #25)
> [Tracking Requested - why for this release]:
> crap, this bug is rolling out in 55.

I am also seeing this in 54 with a new profile if browser.tabs.remote.autostart.2 is set to true. Setting it to false and restarting lets me D-n-D again.

Comment 30

[Stephen Horlander [:shorlander]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #28)
> Try the other one. It works for me on Nightly.

Double Tap + Drag also WFM on Nightly.

Comment 31

[Stephen Horlander [:shorlander]]

(In reply to Stephen Horlander [:shorlander] from comment #30)
> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #28)
> > Try the other one. It works for me on Nightly.
> 
> Double Tap + Drag also WFM on Nightly.

After further tests some notes:

- It works on *some* elements: tab dragging and toolbar customization works
- It *does not* work on other things such as: text selection, new tab thumbnails (they navigate before you can double tab)

Comment 32

[Kartikaya Gupta (email:[e-mail])]

Thanks. I also talked with bwinton on IRC and confirmed that the behaviour as I would expect. Quoting him:

> Firefox-UI works the same as Firefox-Content.
> Chrome-Content just doesn't work at all, but Chrome-UI allows single tap then drag.

Previously, we didn't have proper touch support in Firefox, so we were letting the OS downgrade the touch events to mouse events. So the gesture where you do "finger down, finger move" would be turned into "mouse down, mouse move" and would effectively do a mouse drag. Now that we have proper touch events support, we do a "touch drag" for which the gesture is "double-tap-drag". This is documented at [1] for example, see the "Drag One Finger" vs the "Double-tap and Drag (Tap and a Half)" animation and descriptions.

There is additional context in bug 1328285 and bug 1147335 where this was implemented. IMO the current behaviour is as expected. If we want to support the "finger down, finger move" gesture in specific places like dragging tabs and the customization panel we can do that in the front-end by listening for the relevant touch events. Bug 1299286 comment 2 explains this, and that bug is specifically about supporting this gesture in the customization panel. Bug 1334161 is tracking other touch-friendliness issues, so feel free to hang other bugs off that.

[1] https://www.microsoft.com/surface/en-ca/support/hardware-and-drivers/touchpad-a-builtin-mouse?os=windows-10&=undefined

(Updated post mid-air collision:)

(In reply to Stephen Horlander [:shorlander] from comment #31)
> - It *does not* work on other things such as: text selection, new tab
> thumbnails (they navigate before you can double tab)

These seem like things that should be fixed.

Comment 33

[Stephen Horlander [:shorlander]]

Thanks for the explanation!

After digging into this a little more our behavior does more closely match the default behaviors of Windows 10, even though it is a departure from our previous behavior. There are also a variety of cases/modes where the default behavior is finger down + drag as the default: e.g. the Task View interface or dragging files in File Explorer.

I have concerns around how discoverable double click+drag is going to be on mixed input devices, it's also an awkward and unforgiving gesture (see trying to double click and drag a top site tile / any navigation element that is also draggable).

That investigation led me to notice that double-click + drag is *not* the standard behavior for dragging in Edge or other apps. The default standard for dragging seems to be press + hold + drag.

You can see this on display on Edge's tabs or top site tiles. Press and hold until you get the black square indicator and then start dragging. Or you can see the same gesture for dragging Start Menu tiles.

Aside from calling out modes where dragging is the primary function (Customization) I think we should support the touch + hold + drag interaction and not the double-click + drag interaction.

Comment 34

[Kartikaya Gupta (email:[e-mail])]

(In reply to Stephen Horlander [:shorlander] from comment #33)
> That investigation led me to notice that double-click + drag is *not* the
> standard behavior for dragging in Edge or other apps. The default standard
> for dragging seems to be press + hold + drag.

I did try to implement it that way initially, as that was what Philipp requested in bug 1147335 comment 3. However I ran into problems with the Windows APIs not really allowing that. This is documented somewhat around bug 1147335 comment 19 and the next few comments. Note in particular the links in comment 20 which point to Chromium devs running into the same problem and being unable to solve it. I don't know how Edge does it but they probably have access to secret internal OS things that are either not exposed or not documented.

Comment 35

[Jim Mathies [:jimm]]

Can we just add touch input handling to tabs so that they do proper "finger down, move" tab dragging? That should be pretty easy to implement up in tabbrowser js.

(In reply to Stephen Horlander [:shorlander] from comment #33)
> I have concerns around how discoverable double click+drag is going to be on
> mixed input devices, it's also an awkward and unforgiving gesture (see
> trying to double click and drag a top site tile / any navigation element
> that is also draggable).

Agreed. Firefox feels like a browser that wasn't designed for touch due to this.

Sounds like this bug needs to move to front-end and I need to escalate it there if my first comment is correct.

Comment 36

[Kartikaya Gupta (email:[e-mail])]

(In reply to Jim Mathies [:jimm] from comment #35)
> Can we just add touch input handling to tabs so that they do proper "finger
> down, move" tab dragging? That should be pretty easy to implement up in
> tabbrowser js.

Yup, if that's what we want it should be fairly easy to implement in the frontend code.

> Sounds like this bug needs to move to front-end and I need to escalate it
> there if my first comment is correct.

Agreed.

Comment 37

[Jim Mathies [:jimm]]

Justin, can you help get this injected into the front end team's triage process?

Comment 38

[Justin Dolske [:Dolske]]

Tying this into the Photon Touch work for triage.

Comment 39

[Stephen Horlander [:shorlander]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #34)
> I did try to implement it that way initially, as that was what Philipp
> requested in bug 1147335 comment 3. However I ran into problems with the
> Windows APIs not really allowing that. This is documented somewhat around
> bug 1147335 comment 19 and the next few comments. Note in particular the
> links in comment 20 which point to Chromium devs running into the same
> problem and being unable to solve it. I don't know how Edge does it but they
> probably have access to secret internal OS things that are either not
> exposed or not documented.

That's unfortunate :(


(In reply to Jim Mathies [:jimm] from comment #35)
> Can we just add touch input handling to tabs so that they do proper "finger
> down, move" tab dragging? That should be pretty easy to implement up in
> tabbrowser js.
> 
> (In reply to Stephen Horlander [:shorlander] from comment #33)
> > I have concerns around how discoverable double click+drag is going to be on
> > mixed input devices, it's also an awkward and unforgiving gesture (see
> > trying to double click and drag a top site tile / any navigation element
> > that is also draggable).
> 
> Agreed. Firefox feels like a browser that wasn't designed for touch due to
> this.
> 
> Sounds like this bug needs to move to front-end and I need to escalate it
> there if my first comment is correct.

After poking around at this for a while I think there are a few areas where this change in behavior has caused some undesirable regressions in primary UI:

- Tab D-n-D
- Customization Mode D-n-D
- New Tab tiles D-n-D
- Bookmarks / Library window D-n-D

Comment 40

[Julien Cristau [:jcristau]]

Guessing P3 means this isn't a priority to get fixed in 55.

Comment 41

[Kartikaya Gupta (email:[e-mail])]

Note that Johann has been working on a solution to this problem in bug 1299286. His solution should allow fixing this bug as well.

Comment 42

[Johann Hofmann [:johannh]]

I think we've actually gotten close enough in bug 1299286 to block this. I'll be on PTO beginning of next week but fixing this is my priority right now.

Comment 43

[Johann Hofmann [:johannh]]

Attachment: Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

This allows dragging them via touch gestures.

Review commit: https://reviewboard.mozilla.org/r/198584/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198584/

Comment 44

[Dão Gottwald [:dao]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

https://reviewboard.mozilla.org/r/198584/#review203718

::: browser/base/content/tabbrowser.xml:7856
(Diff revision 1)
>            else
>              this.removeAttribute("visuallyselected");
> +
> +          // On Windows, enable touch events to start a native dragging
> +          // session to allow the user to easily drag the selected tab.
> +          if (AppConstants.platform == "win") {

Why this check? Is the attribute harmful on other platforms, rather than a no-op?

::: browser/base/content/tabbrowser.xml:7860
(Diff revision 1)
> +          // session to allow the user to easily drag the selected tab.
> +          if (AppConstants.platform == "win") {
> +            if (val) {
> +              this.setAttribute("touchdownstartsdrag", "true");
> +            } else {
> +              this.removeAttribute("touchdownstartsdrag");

Can you move this to updateCurrentBrowser?

Comment 45

[Johann Hofmann [:johannh]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

https://reviewboard.mozilla.org/r/198584/#review203718

> Why this check? Is the attribute harmful on other platforms, rather than a no-op?

It's a no-op, if you prefer to always have the attribute I can remove the check and add a comment. :)

> Can you move this to updateCurrentBrowser?

Done! Patch coming up in a second...

Comment 46

[Johann Hofmann [:johannh]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198584/diff/1-2/

Comment 47

[Dão Gottwald [:dao]]

Can you also remove https://searchfox.org/mozilla-central/rev/30ead7d1ae5bf95b8bc0fd67b950cd46ca05e32c/browser/components/customizableui/CustomizeMode.jsm#907 while you're at it?

Comment 48

[Johann Hofmann [:johannh]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198584/diff/2-3/

Comment 49

[Johann Hofmann [:johannh]]

Done.

Comment 50

[Dão Gottwald [:dao]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

https://reviewboard.mozilla.org/r/198584/#review204460

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa8ae0bf66d6
Add the touchdownstartsdrag attribute to selected tabs on Windows. r=dao

Comment 52

[Noemi Erli[:noemi_erli]]

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

Comment 53

[Johann Hofmann [:johannh]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

Approval Request Comment
[Feature/Bug causing the regression]: Shipping touch events in bug 1244402
[User impact if declined]: Users are not able to simply drag their tabs using touch gestures. The fix was intended to be part of Photon to emphasize touch friendliness. It would be good to have it in 58 at least.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: It works for me on Nightly
[Needs manual test from QE? If yes, steps to reproduce]: Use a touchscreen and try to drag the selected tab.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: The slightly more risky work for touch dragging is already in 58 through bug 1299286, this patch simply adds a DOM attribute that enables this functionality for tabs.
[String changes made/needed]: None

Comment 54

[Gerry Chang [:gchang]]

Hi :shorlander, 
Can you help check if this issue was fixed in the latest nightly?

Comment 55

[Blake Winton (:bwinton) (:☕️)]

Works for me on 59.0a1 (2017-11-19) (64-bit), but only on the active tab, and fails when the active tab is the Customize Mode tab.
(Actually, dragging that with the mouse also fails, so maybe that's okay…)

Comment 56

[Gerry Chang [:gchang]]

Comment on attachment 8927270 [details]
Bug 1362065 - Add the touchdownstartsdrag attribute to selected tabs on Windows.

Fix a touch gestures issue. Beta58+.

Comment 57

[Johann Hofmann [:johannh]]

(In reply to Blake Winton (:bwinton) (:☕️) from comment #55)
> Works for me on 59.0a1 (2017-11-19) (64-bit), but only on the active tab,
> and fails when the active tab is the Customize Mode tab.

The active tab part is on purpose, see commit message. I found it a little difficult to handle both scrolling the tab bar and dragging tabs using touch. If you're not happy with this we could consider setting the attribute on all tabs when there's no overflow. You can actually play around with this quite easily by setting touchdownstartsdrag=true on any element (or parent element for a group of elements) that should be draggable through touch, e.g. set it on #tabbrowser-tabs.

If you think we can improve something, feel free to make a new bug.

> (Actually, dragging that with the mouse also fails, so maybe that's okay…)

Yeah :)

Comment 58

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 59

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce the issue described in comment 0 using Firefox 55.0a1 (BuildId:20170504030320).

This issue is verified fixed on Firefox 59.0a1 (BuildId:20171121100129) and Firefox 58.0b5 (20171120142222) using Windows 10 64bit on a Dell XPS 12-9Q33.