Automatically enable Firefox touch mode in Windows Tablet mode

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
4 months ago
14 days ago

People

(Reporter: dao, Assigned: johannh)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 56
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

(Whiteboard: [photon-visual][p2][57])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Updated

4 months ago
status-firefox55: affected → ---
(Reporter)

Updated

4 months ago
Depends on: 1355772

Updated

4 months ago
Flags: qe-verify+
QA Contact: ovidiu.boca
(Reporter)

Updated

4 months ago
Whiteboard: [photon-visual][57] → [photon-visual][p2][57]

Updated

4 months ago
QA Contact: ovidiu.boca → brindusa.tot
(Reporter)

Updated

4 months ago
OS: All → Windows
Summary: Automatically enable touch mode when the user is using the touch screen → Automatically Firefox enable touch mode in Windows Tablet mode
(Reporter)

Updated

3 months ago
Summary: Automatically Firefox enable touch mode in Windows Tablet mode → Automatically enable Firefox touch mode in Windows Tablet mode
(Reporter)

Updated

3 months ago
Depends on: 1365912
(Reporter)

Updated

3 months ago
Blocks: 1170714
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED

Updated

3 months ago
Iteration: --- → 55.7 - Jun 12
Priority: P2 → P1
Comment hidden (mozreview-request)
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.
Flags: needinfo?(shorlander)

Updated

2 months ago
Iteration: 55.7 - Jun 12 → 56.1 - Jun 26
Aaron is going to look into this. Redirecting the NI.
Flags: needinfo?(shorlander) → needinfo?(abenson)

Comment 4

2 months ago
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.
Flags: needinfo?(abenson)
That is perfect, thank you!
Attachment #8875273 - Flags: review?(dao+bmo)
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.
(Reporter)

Comment 7

2 months ago
mozreview-review
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?
Attachment #8875273 - Flags: review?(dao+bmo) → review-
Comment hidden (mozreview-request)
Good points, thank you.
(Reporter)

Comment 10

2 months ago
mozreview-review
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.
Attachment #8875273 - Flags: review?(dao+bmo)
Comment hidden (mozreview-request)
(Reporter)

Comment 12

2 months ago
mozreview-review
Comment on attachment 8875273 [details]
Bug 1355771 - Automatically enable Firefox touch mode in Windows Tablet mode.

https://reviewboard.mozilla.org/r/146674/#review153462
Attachment #8875273 - Flags: review?(dao+bmo) → review+

Comment 13

2 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/447ee2268678
Automatically enable Firefox touch mode in Windows Tablet mode. r=dao
https://hg.mozilla.org/mozilla-central/rev/447ee2268678
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
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?
Flags: needinfo?(jhofmann)
(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!
Flags: needinfo?(jhofmann)
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.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.