Closed Bug 1322349 Opened 3 years ago Closed 3 years ago

With touch enabled, swipe gesture (Windows 10 Insider Preview) doesn't bring down toolbar in fullscreen mode - actionable once released version supports swipe, since 52 is ESR

Categories

(Core :: Panning and Zooming, defect, P2)

52 Branch
All
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + verified
firefox53 --- verified
firefox54 --- verified

People

(Reporter: petruta.rasa, Assigned: kats)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(1 file)

[Affected versions]:
- Aurora 52.0a2 2016-12-06
- Nightly 53.0a1 2016-12-06

[Affected platforms]:
- Microsoft Surface Pro with Windows 10 Pro Insider Preview

[Steps to reproduce]:
1. Tap the Location bar and use the onscreen keyboard to type about:support
2. Tap the Menu icon and select "Full screen" option
3. While in full-screen mode, swipe vertically the top of the screen to bring up the toolbar

[Expected result]:
- Toolbar is visible and other actions can be performed

[Actual result]:
- Nothing happens, there is no way to exit fullscreen. 

[Regression range]:
- https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4891238709e17274db6ffa4880730b97ff6c24da&tochange=888c1fb24f815739efe0fb7d7ae4260aaffce746

[Additional notes]:
- Reproducible on Firefox 51 beta 6 too with e10s enabled and dom.w3c_touch_events.enabled set to 2
I'll look into this next week when I have access to my Windows surface device again. Parking with me for now. Does the escape key not work to get out of fullscreen in this scenario?
Assignee: nobody → bugmail
Priority: -- → P2
Whiteboard: [gfx-noted]
I have no Escape key available on the on-screen keyboard although I did checked "Add the standard keyboard layout as a touch keyboard option".

I can exit if I have a mouse attached (the Toolbar is displayed when the mouse icon is reaching the top of the page) or by swiping left to switch apps and exiting Firefox.
A few questions: when you repro this bug, can you confirm that you have unplugged all physical keyboard/mice? And is your windows OS in "tablet mode" or not when you see this issue? Finally, can you repro only on the Insider Preview OS, or on a regular Windows 10 Pro as well?

I'm on Windows 10 Pro (not the Insider Preview) on my Surface Pro. When I try to follow your STR (in 51b7, or in latest nightly 53) swiping vertically does nothing for me. Or rather, most of the time it does nothing, and intermittently it does something unexpected.

Instead, for me the gesture that works to make the toolbar visible in fullscreen mode is *tapping* the bezel just above the screen area (for example between the camera lens and the visible screen area, but anyplace just above the screen works). This gesture works consistently in 51b7 (e10s disabled) and latest Nightly (e10s+touch enabled). It works in both tablet mode and without tablet mode.
Flags: needinfo?(petruta.rasa)
I put a video of it working for me at https://people.mozilla.org/~kgupta/tmp/bezel_tap.mp4 if it helps clarify what I'm doing.
Thanks, tapping that area works for me too, no matter if I'm in tablet or desktop mode. I didn't knew that trick.

I have no physical keyboard, only a mice, having it plugged or unplugged makes no difference.
We only have one device available and it only has Insider Preview build on it.

In older versions both methods (swiping vertically and tapping the bezel) used to work. 
Should I further investigate or is this issue invalid in this case? Thank you!
Has Regression Range: --- → yes
Flags: needinfo?(petruta.rasa)
Keywords: regression
Hm, I guess the swipe gesture might be new in the Insider Preview, and we might need to add support for that in Firefox. However it seems like that gesture is not a thing in Windows 10 Pro (non-Insider Preview). So I say we leave this bug open but I wouldn't consider it a blocker for touch events since it's not a regression against released OS versions.
Summary: Cannot exit Fullscreen mode with touch enabled → With touch enabled, swipe gesture (Windows 10 Insider Preview) doesn't bring down toolbar in fullscreen mode
Unassigning for now. If the swipe gesture becomes makes it into a Windows 10 Pro release build then we can pick this up again.
Assignee: bugmail → nobody
Summary: With touch enabled, swipe gesture (Windows 10 Insider Preview) doesn't bring down toolbar in fullscreen mode → With touch enabled, swipe gesture (Windows 10 Insider Preview) doesn't bring down toolbar in fullscreen mode - actionable once released version supports swipe, since 52 is ESR
The Win10 Creator update is slated for March, which would coincide very closely with the release of Firefox 52. And IIRC, Firefox 52 is also the release we're targeting shipping Windows touchscreen support in. It would be really nice if we didn't have to scramble to fix and backport this at the last second if we know about it now. Note that build 14986 was recently released to the Slow Ring for testing as well, which is a wider audience and generally considered more stable.
Flags: needinfo?(bugmail)
Once I'm done with bug 1147335 I'll try to install a preview build and repro this bug. Leaving ni on me for now.
I can repro on Insider Preview. I'll investigate.
Assignee: nobody → bugmail
Flags: needinfo?(bugmail)
The problem here is pretty simple. The actual reason the toolbar shows up appears to be because of mousemove events at y=0 or y=1. When we have touch events enabled, the mousemove events that are normally generated from an edge swipe are suppressed because of the code at [1]. With a "tap" (as I did in the video in comment 4) we generate a synthetic mouse move. I guess I need to find the place that is listening for these mousemove events and also make it listen for touchmove events.

[1] http://searchfox.org/mozilla-central/rev/02a56df6474a97cf84d94bbcfaa126979970905d/widget/windows/nsWindow.cpp#4259
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #12)
> It's here:
> https://dxr.mozilla.org/mozilla-central/rev/
> 8ff550409e1d1f8b54f6f7f115545dbef857be0b/browser/base/content/browser-
> fullScreenAndPointerLock.js#297

Thanks. Adding a touchmove listener here seems to do the job but then it becomes hard to "hide" the url bar and go back into fullscreen. With the mouse moving the mouse down into the mouse target area [1] accomplishes this. With touch it's not so obvious how to do - this is partly a UX problem as well. Right now if you have a chrome-process page (e.g. about:support) loaded and tap on the content area it will dispatch a mouse event in the UI process which will trigger the existing MousePosTracker/onEnter code and accomplish the desired effect. However if you have a real webpage in a content process the UI process never gets the mouse event (it is generated in the content process) and so the URL bar remains visible. I guess we want to also have MousePosTracker listen for touchstart events, or something along those lines.

[1] http://searchfox.org/mozilla-central/rev/7da3c9dcf467964f2fb82f3a4c63972ee79bf696/browser/base/content/browser-fullScreenAndPointerLock.js#571
Listening for touch events in MousePosTracker doesn't work, because the touch events go to the content process. Maybe we have chrome code running in the content process that we can use? I'm not sure. For now I'm going to fix the swipe action bringing back the toolbar, since I think being able to escape fullscreen is pretty important. I can file a follow-up for the remaining issues (both re-entering fullscreen and for the DOM fullscreen warning box thing).

I think we should probably get some input from UX on what to do here as well. My instinct of just having a "tap" on the content area go back into fullscreen might be bad UX because the user might be trying to tap on something and it's not clear what should happen there.
See Also: → 1334173
See Also: → 1334179
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> I can file a follow-up for the
> remaining issues (both re-entering fullscreen and for the DOM fullscreen
> warning box thing).
> 

Filed bug 1334179 and bug 1334173, respectively.
Comment on attachment 8830783 [details]
Bug 1322349 - Allow a touch swipe gesture at the top of the screen to show the URL bar in fullscreen mode.

https://reviewboard.mozilla.org/r/107502/#review108652
Attachment #8830783 - Flags: review?(jaws) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ce0f2b0b8e7
Allow a touch swipe gesture at the top of the screen to show the URL bar in fullscreen mode. r=jaws
https://hg.mozilla.org/mozilla-central/rev/6ce0f2b0b8e7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Tentatively tracking for 52.  Patch looks simple enough for uplift up to beta once it proves itself in nightly.
Rares, can you please punt this over to whoever is doing touchscreen QA to verify the fix on nightly before we request uplift? Note that it requires a Windows 10 Insider build to test.
Flags: needinfo?(rares.bologa)
Flags: needinfo?(rares.bologa) → needinfo?(petruta.rasa)
Verified on Nightly 54.0a1 2017-01-30 that swipe gesture brings the toolbar back. Tapping the Menu Panel twice (or Menu Panel + outside it) re-enters Full Screen mode.

If there are several tabs opened, it's possible to open a tab in a new window when swiping down. This wasn't introduced by this patch as it happens on previous versions too. I will add this as a note in bug 1334179.
Status: RESOLVED → VERIFIED
Flags: needinfo?(petruta.rasa)
Comment on attachment 8830783 [details]
Bug 1322349 - Allow a touch swipe gesture at the top of the screen to show the URL bar in fullscreen mode.

Approval Request Comment
[Feature/Bug causing the regression]: Windows touch events support (bug 1180706 on nightly, bug 1244402 on aurora/beta)
[User impact if declined]: when in fullscreen mode, doing a swipe downwards at the top of the screen doesn't make the url bar visible like it used to
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: STR are in comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: one-line fix that allows the fullscreen code to pick up on touch events as well as mouse events.
[String changes made/needed]: none
Attachment #8830783 - Flags: approval-mozilla-beta?
Attachment #8830783 - Flags: approval-mozilla-aurora?
Comment on attachment 8830783 [details]
Bug 1322349 - Allow a touch swipe gesture at the top of the screen to show the URL bar in fullscreen mode.

fix for fullscreen mode on touchscreens, aurora53+, beta52+
Attachment #8830783 - Flags: approval-mozilla-beta?
Attachment #8830783 - Flags: approval-mozilla-beta+
Attachment #8830783 - Flags: approval-mozilla-aurora?
Attachment #8830783 - Flags: approval-mozilla-aurora+
needs rebasing for beta

warning: conflicts while merging browser/base/content/browser-fullScreenAndPointerLock.js! (edit, then use 'hg resolve --mark')
Flags: needinfo?(bugmail)
Verified as fixed using Firefox 52 beta 3 and latest Aurora 53.0a2 2017-02-05.
You need to log in before you can comment on or make changes to this bug.