1355771 - Automatically enable Firefox touch mode in Windows Tablet mode

Status: VERIFIED FIXED

(Firefox :: Theme, enhancement, P1)

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

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

Comment 2

[Johann Hofmann [:johannh]]

Stephen,

talking to Dao made me aware that the solution I'm proposing probably needs input from UX.

The density selector from the spec (https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/229252092) doesn't cover the case when we override the default/customized UI density because we're in Windows Tablet Mode. Should the UI reflect that we have overridden the preference temporarily and will restore the old density as soon as Tablet Mode is turned off? What would that look like?

My patch also adds a pref that allows the user to disable that behavior, which assumes that there will be some sort of checkbox in the UI to disable it. Are we adding a setting to disable this behavior for Windows users? It sounds useful to me.

I will implement the actual UI in bug 1350210 afterwards, but these details would be good to know to correctly represent the UI density internally.

Comment 3

[Stephen Horlander [:shorlander]]

Aaron is going to look into this. Redirecting the NI.

Comment 4

[Aaron Benson]

I've updated the spec (linked above) and added detail in the Explainer Sheet (scroll down to 'UI Density Selector') https://mozilla.invisionapp.com/share/WUBRB21NH#/229252091_Customization

There is now a checkbox in the flyout, enabled by default, that automatically switches to the Touch-friendly density setting when Windows is in Tablet mode. You can select a new density setting in tablet mode, if you wish, and the checkbox should uncheck itself.

Comment 5

[Johann Hofmann [:johannh]]

That is perfect, thank you!

Comment 6

[Johann Hofmann [:johannh]]

Dao, I believe my changes conform to this spec (there's a global pref for automatically switching to touch density and it doesn't affect the regular uidensity pref). Let me know what you think.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

https://reviewboard.mozilla.org/r/146674/#review153400

::: browser/app/profile/firefox.js:241
(Diff revision 1)
>  // UI density of the browser chrome. This mostly affects toolbarbutton
>  // and urlbar spacing. The possible values are 0=normal, 1=compact, 2=touch.
>  pref("browser.uidensity", 0);
> +// Whether Firefox will automatically override the uidensity to "touch"
> +// while the user is in a touch environment (such as Windows tablet mode).
> +pref("browser.touchmode.auto", false);

enable this for MOZ_PHOTON_THEME?

::: browser/base/content/browser.js:5397
(Diff revision 1)
>    update(isInTabletMode) {
>      let wasInTabletMode = document.documentElement.hasAttribute("tabletmode");
>      if (isInTabletMode) {
>        document.documentElement.setAttribute("tabletmode", "true");
> +      if (gPrefService.getBoolPref("browser.touchmode.auto")) {
> +        gUIDensity.update(2);

I see two footguns here:

1. This depends on TabletModeUpdater.init running after gUIDensity.update in gBrowserInit.onLoad.

2. Calling gUIDensity.update again would revert the UI density to what it was before despite browser.touchmode.auto=true.

Can we make gUIDensity.update query TabletModeUpdater instead?

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

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

Comment 9

[Johann Hofmann [:johannh]]

Good points, thank you.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

https://reviewboard.mozilla.org/r/146674/#review153452

::: browser/base/content/browser.js:5459
(Diff revision 2)
>  
>      this.update();
>    },
>  
>    update() {
> +    let pref;

s/pref/mode/

::: browser/base/content/browser.js:5462
(Diff revision 2)
>  
>    update() {
> +    let pref;
> +    // Automatically override the uidensity to touch in Windows tablet mode.
> +    if (AppConstants.isPlatformAndVersionAtLeast("win", "10") &&
> +        WindowsUIUtils.inTabletMode && gPrefService.getBoolPref("browser.touchmode.auto")) {

nit: break the line after &&

::: browser/base/content/browser.js:5463
(Diff revision 2)
>    update() {
> +    let pref;
> +    // Automatically override the uidensity to touch in Windows tablet mode.
> +    if (AppConstants.isPlatformAndVersionAtLeast("win", "10") &&
> +        WindowsUIUtils.inTabletMode && gPrefService.getBoolPref("browser.touchmode.auto")) {
> +      pref = 2;

Let's add gUIDensity.MODE_COMPACT and gUIDensity.MODE_TOUCH and use them here instead of magic numbers.

Comment 11

[Johann Hofmann [:johannh]]

Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

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

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

https://reviewboard.mozilla.org/r/146674/#review153462

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/447ee2268678
Automatically enable Firefox touch mode in Windows Tablet mode. r=dao

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/447ee2268678

Comment 15

[ovidiu boca[:Ovidiu]]

Hi, from what I understood, reading the bug, here we need to verify if the "Use Touch for Table Mode" is enabled by default? 	Johann, can you please help me with this and tell me if that's correct?

Comment 16

[Johann Hofmann [:johannh]]

(In reply to ovidiu boca[:Ovidiu] from comment #15)
> Hi, from what I understood, reading the bug, here we need to verify if the
> "Use Touch for Table Mode" is enabled by default? 	Johann, can you please
> help me with this and tell me if that's correct?

You'll need to verify that, using Windows 10, if you turn on Windows Tablet Mode (https://www.laptopmag.com/articles/enable-tablet-mode-windows-10) Firefox automatically switches into Firefox Touch Mode (buttons are larger).

Thanks!

Comment 17

[ovidiu boca[:Ovidiu]]

I verified this on Windows 10 x64 with FF Nightly 57.0a1(2017-08-09) and I can confirm the fix. Thanks Johann for your help.