Open Bug 1875079 Opened 5 months ago Updated 2 months ago

Apply touchmode rules to all OSes

Categories

(Firefox :: Menus, enhancement)

enhancement

Tracking

()

ASSIGNED

People

(Reporter: saschanaz, Assigned: saschanaz)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

No description provided.
Assignee: nobody → krosylight
Status: NEW → ASSIGNED

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

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

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e000fc4c30e
Always enable css zoom in chrome pages. r=saschanaz
Keywords: leave-open
Pushed by dgottwald@mozilla.com:
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
Regressions: 1875863

Backed out for causing bc failures on browser_menu_touch.js

Backout link

Push with failures

Failure log

Flags: needinfo?(krosylight)

And also the issue in bug 1875863.

Flags: needinfo?(krosylight)
Attachment #9373250 - Attachment description: Bug 1875079 - Part 2: Reduce max-height by zoom factor r=#desktop-theme-reviewers → Bug 1875079 - Part 2: 1.3x zoom for touchmode r=#desktop-theme-reviewers.

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

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.

Flags: needinfo?(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?

Flags: needinfo?(emilio) → needinfo?(krosylight)

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

Flags: needinfo?(krosylight)

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

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

Flags: needinfo?(krosylight)

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?

Flags: needinfo?(krosylight) → needinfo?(emilio)

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

Adding a currentCSSZoomWithoutFlushing or something makes sense to me.

Flags: needinfo?(emilio)

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: