Closed Bug 1888242 Opened 1 year ago Closed 1 year ago

Entering and Exiting Responsive Mode Can Result in Busted Scrollport

Categories

(DevTools :: Responsive Design Mode, defect, P2)

Product:
DevTools
Component:
Responsive Design Mode
Type:
defect

Tracking

(firefox-esr115 unaffected, firefox124 wontfix, firefox125 wontfix, firefox126 verified, firefox127 verified)

Status:
VERIFIED FIXED
Milestone:
126 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox124 --- wontfix
firefox125 --- wontfix
firefox126 --- verified
firefox127 --- verified

People

(Reporter: dshin, Assigned: emilio)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Screen capture of the busted behaviour on Google
3.46 MB, video/webm
Details
Bug 1888242 - Simplify viewport handling in RDM. r=#devtools-reviewers,bradwerth
48 bytes, text/x-phabricator-request
Details | Review

STR:

  • Load the following testcase:
data:text/html,<!DOCTYPE html><style>:root { line-height: 50em }</style>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>
  • Open Inspector & Responsive Mode
  • Set the device to something (Used iPhone 12/13 Pro Max in the screen capture, but others seem to work)
  • Exit Responsive Mode by pressing the X button on the top right

Expected: Viewport reverts to the original state
Actual: There seem to be some sort of race condition, so this doesn't happen 100% of the time, but the viewport is ~1/4x the size, until the inspector closes as well. Scrolling seems busted as well (See screen capture).
Doesn't not seem to happen on MDN/if you add the meta viewport tag to the testcase, <meta name="viewport" content="width=device-width,initial-scale=1">

mozregression --arg=https://www.google.com/search?q=css --repo=autoland --good=41b40b70dd9f7013eb41f618863e07d413194319 --bad=08da3ca498f61eb9acb594038c5e764cfb37d735
Keywords: regression
Regressed by: 1847584

Set release status flags based on info from the regressing bug 1847584

:emilio, since you are the author of the regressor, bug 1847584, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

status-firefox124: --- → affected
status-firefox125: --- → affected
status-firefox126: --- → affected
status-firefox-esr115: --- → unaffected
Flags: needinfo?(emilio)

Ah, so I can repro but only if devtools is somehow also open. Does that match your experience?

It seems the mobile viewport gets stuck: innerWidth is 980 and innerHeight is 258.

Greg, curious, which patch in particular from that bug causes the regression? I think in practice it likely just exposed an existing race but I might be missing something obvious.

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

As expected it doesn't happen with:

data:text/html,<!DOCTYPE html><meta name=viewport content="width=device-width"><style>:root { line-height: 50em }</style>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>a<br>b<br>
See Also: → 1569626, 1625999

In particular:

  • Always handle meta viewport in RDM. This fixes bug 1625999 too by
    making touch simulation enabled and disabled consistent.

  • Restore the resolution to 1 when toggling RDM. This is just simpler,
    and we're not keeping around the visual viewport offsets anyways
    so...

  • Deal with the change more easily, at the same point we switch
    scrollbars etc.

Assignee: nobody → emilio
Status: NEW → ASSIGNED

Would be good to check comment 5 also fixes it for you, since this is a bit racy, if you have the time...

Flags: needinfo?(dshin)

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

Greg, curious, which patch in particular from that bug causes the regression? I think in practice it likely just exposed an existing race but I might be missing something obvious.

https://hg.mozilla.org/integration/autoland/rev/08da3ca498f61eb9acb594038c5e764cfb37d735

Flags: needinfo?(gregp)

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

Would be good to check comment 5 also fixes it for you, since this is a bit racy, if you have the time...

Confirming that I can reproduce this on local opt build pre-patch and cannot reproduce post-patch.

Flags: needinfo?(dshin)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/13e37e36d1c6 Simplify viewport handling in RDM. r=bradwerth,devtools-reviewers,ochameau
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/8dde0280687a Fix a couple tests now that we always support meta viewport in RDM.
Severity: -- → S3
Priority: -- → P2

https://hg.mozilla.org/mozilla-central/rev/13e37e36d1c6
https://hg.mozilla.org/mozilla-central/rev/8dde0280687a
https://hg.mozilla.org/mozilla-central/rev/0626e6c46d2a

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox126: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox125 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)
Duplicate of this bug: 1625999
status-firefox125: affected → wontfix
Flags: needinfo?(emilio)

Issue is reproducible on a 2024-03-27 Nightly build on Windows 10.
Verified as fixed on Firefox Nightly 127.0a1 and Firefox 126.0 on Windows 10, Ubuntu 22, macOS 12.

Status: RESOLVED → VERIFIED
status-firefox126: fixed → verified
status-firefox127: --- → verified
Flags: qe-verify+
Regressions: 1920357
Regressions: 1931482
No longer regressions: 1931482
See Also: → 1974433
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: