Petruta Horea [:phorea], Desktop QA Reporter
	Description • 8 years ago

[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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 1 • 8 years ago

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?

	Petruta Horea [:phorea], Desktop QA Reporter
	Comment 2 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 3 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 4 • 8 years ago

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.

	Petruta Horea [:phorea], Desktop QA Reporter
	Comment 5 • 8 years ago

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!

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 6 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 7 • 8 years ago

Unassigning for now. If the swipe gesture becomes makes it into a Windows 10 Pro release build then we can pick this up again.

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 9 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 10 • 8 years ago

I can repro on Insider Preview. I'll investigate.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 11 • 7 years ago

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

	Xidorn Quan [:xidorn] UTC+11
	Comment 12 • 7 years ago

It's here: https://dxr.mozilla.org/mozilla-central/rev/8ff550409e1d1f8b54f6f7f115545dbef857be0b/browser/base/content/browser-fullScreenAndPointerLock.js#297

You may want to also do something here for the warning box of DOM Fullscreen: https://dxr.mozilla.org/mozilla-central/rev/8ff550409e1d1f8b54f6f7f115545dbef857be0b/browser/base/content/browser-fullScreenAndPointerLock.js#62

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 13 • 7 years ago

(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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 14 • 7 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 16 • 7 years ago

(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.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 17 • 7 years ago
mozreview-review

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

	Pulsebot
	Comment 18 • 7 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 19 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6ce0f2b0b8e7

	Julien Cristau [:jcristau]
	Comment 20 • 7 years ago

Tentatively tracking for 52.  Patch looks simple enough for uplift up to beta once it proves itself in nightly.

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 7 years ago

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.

	Petruta Horea [:phorea], Desktop QA Reporter
	Comment 22 • 7 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 23 • 7 years ago

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

	Julien Cristau [:jcristau]
	Comment 24 • 7 years ago

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+

	Carsten Book [:Tomcat]
	Comment 25 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/87ff239bdeb8

	Carsten Book [:Tomcat]
	Comment 26 • 7 years ago

needs rebasing for beta

warning: conflicts while merging browser/base/content/browser-fullScreenAndPointerLock.js! (edit, then use 'hg resolve --mark')

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 27 • 7 years ago

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

	Petruta Horea [:phorea], Desktop QA Reporter
	Comment 28 • 7 years ago

Verified as fixed using Firefox 52 beta 3 and latest Aurora 53.0a2 2017-02-05.