1875079 - Apply touchmode rules to all OSes

Status: ASSIGNED

(Firefox :: Menus, enhancement)

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1875079 - Part 1: Toggle touchmode from nsXULPopupManager r=#desktop-theme-reviewers

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1875079 - Part 2: 1.3x zoom for touchmode r=#desktop-theme-reviewers.

Revert "Backed out changeset 3d1ef5fe232a (bug 1874301) as requested by Kagami for causing css overflow issue that blocks accessing app menu on certain devices."

This reverts commit 74b2316865d9b72ddcc62ede4933bf972847017e.

Depends on D198801

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1875079 - Always enable css zoom in chrome pages. r=saschanaz

Without this beta simulation jobs will fail with the previous patches.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e000fc4c30e
Always enable css zoom in chrome pages. r=saschanaz

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/3e000fc4c30e

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a62cca5ea26
Part 1: Toggle touchmode from nsXULPopupManager r=desktop-theme-reviewers,dao,emilio
https://hg.mozilla.org/integration/autoland/rev/015e7b62138b
Part 2: Reduce max-height by zoom factor r=desktop-theme-reviewers,dao

Comment 7

[Norisz Fay [:noriszfay]]

Backed out for causing bc failures on browser_menu_touch.js

Backout link

Push with failures

Failure log

Comment 8

[Kagami Rosylight [:saschanaz] (they/them)]

And also the issue in bug 1875863.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1875079 - Part 3: Reduce absolute sizes by zoom factor r=#desktop-theme-reviewers

Those sizes are affected by zoom again, so they need to be decreased.

Comment 10

[Kagami Rosylight [:saschanaz] (they/them)]

The third patch is full of hacks and still it has test failures that I can't really understand. https://treeherder.mozilla.org/jobs?repo=try&revision=de77c1989d4106a5a6819a0ca776c6197bf44f98&selectedTaskRun=YqoWjCTfQW6wRS-11pI2Pg.0

Emilio, any idea how to make it less hacky with zoom? Otherwise I think I'd rather go for simply moving the rules rather than adding hacks.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

I'm confused, how does your code in comment 9 and so have anything to do with those failures? The zoom factor should effectively always be 1 there, right? Do the failures reproduce if you just flush style some other way like calling getComputedStyle(this._panel).fontSize or something?

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

If I revert comment #9 those failures go away, I'll check with getComputedStyle.

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Okay I updated the patch, there was a mistake that was somehow silently causing failures.

It's still hacky but I think it should work for now...

Comment 14

[:Gijs (he/him)]

What's needed to move forward here? :-)

Comment 15

[Kagami Rosylight [:saschanaz] (they/them)]

Emilio wanted to have a better way of getting zoom level which is now specced and implemented as currentCSSZoom, but it flushes. Maybe we can add a non-flushing chrome-only attribute, or even zoom-agnostic getBounds function.

Thoughts, Emilio?

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Zoom-agnostic getBounds is getBoundingClientRect or the equivalent non-flushing functions right?

Adding a currentCSSZoomWithoutFlushing or something makes sense to me.

Comment 17

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #16)

Zoom-agnostic getBounds is getBoundingClientRect or the equivalent non-flushing functions right?

Yeah, the reason we need zoom level right now is to reduce the width/height returned by getBounding, so perhaps we can have a helper function instead of exposing non-flush zoom attribute.

Adding a currentCSSZoomWithoutFlushing or something makes sense to me.

But yeah this works for me too.