Closed Bug 1538681 Opened 6 years ago Closed 6 years ago

Scroll bar out of place after rotating from landscape to portrait

Categories

(Firefox for Android Graveyard :: General, defect, P2)

Product:
Firefox for Android Graveyard
Component:
General
Version:
Firefox 68
Platform:
ARM
Android
Type:
defect

Tracking

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 unaffected, firefox68 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 68
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected
firefox68 --- verified

People

(Reporter: csheany, Assigned: bradwerth)

References

(Blocks 1 open bug, Regression,
URL
)

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Screenshot_20190325-090037.jpg
(deleted), image/jpeg
Details
Screenshot_20190325-090621.jpg
(deleted), image/jpeg
Details
Bug 1538681 Part 1: Backout a change that added an early-exit in MobileViewportManager::UpdateResolution.
(deleted), text/x-phabricator-request
Details
Bug 1538681 Part 2: Add a perma-fail test that scrollbars appear in proper location after resizing viewport.
(deleted), text/x-phabricator-request
Details

User Agent: Mozilla/5.0 (Android 7.1.1; Tablet; rv:68.0) Gecko/68.0 Firefox/68.0

Steps to reproduce:

  1. Open https://github.com/mozilla-mobile
  2. Request desktop site
  3. Rotate to landsacpe
  4. Rotate to portrait

Actual results:

The scroll bar appears higher than it is supposed to

Expected results:

The scroll bar should be closer to the edge

Also with following STR,

  1. Open https://github.com/mozilla-mobile
  2. Zoom in
  3. Rotate to landscape

Actual results:

The scroll bar appears...

Expected results:

The scroll bar should be closer to the edge

Summary: Scroll bar out of place after rotating from lanscape to portrait → Scroll bar out of place after rotating from landscape to portrait

Was this caused by Bug 1536755

See also Bug 1511375

Flags: needinfo?(botond)
Attached image Screenshot_20190325-090037.jpg (deleted) —
Attached image Screenshot_20190325-090621.jpg (deleted) —

Caused by Bug 1536755. I'll figure it out.

Assignee: nobody → bwerth
Flags: needinfo?(botond)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

I been trying to reproduce using our Responsive Design Mode on desktop, and have not been successful yet. In doing that, I found a zoom issue with this testcase that is probably part of the solution. I've posted that here while I work on reproducing on Android.

Is the zoom issue you are referring to Bug 1523844/ Bug 1519013?

(In reply to csheany from comment #8)

Is the zoom issue you are referring to Bug 1523844/ Bug 1519013?

Yes, I think my patch is probably addressing Bug 1523844. I'll move the patch over there.

Attachment #9053336 - Attachment is obsolete: true
Depends on: 1523844
URL: https://github.com/mozilla-mobile
Has STR: --- → yes
status-firefox67: --- → unaffected
status-firefox68: --- → affected
Keywords: regression
OS: Unspecified → Android
Hardware: Unspecified → ARM
Priority: -- → P2

Hey Brad, can we get a status update on this one?

Flags: needinfo?(bwerth)

It's dependent on Bug 1523844; I'll add a comment on that Bug detailing where it's stuck.

Flags: needinfo?(bwerth)

(In reply to Brad Werth [:bradwerth] from comment #11)

It's dependent on Bug 1523844; I'll add a comment on that Bug detailing where it's stuck.

I just attempted to again reproduce Bug 1523844 using my Responsive Design Mode setup and I wasn't able to do it. Something has changed -- not claiming the bug is fixed though I would welcome any retest. I need to move to a proper GeckoView test myself for my next attempt at reproduction.

(In reply to Brad Werth [:bradwerth] from comment #5)

Caused by Bug 1536755. I'll figure it out.

Brad, what makes you believe this?

Bug 1536755 landed on 2019-03-25, but I can reproduce this issue in the 2019-03-24 nightly.

Let's get a regression window to see what actually regressed this.

Keywords: regressionwindow-wanted

Maybe Bug 1501665?

(In reply to csheany from comment #15)

Maybe Bug 1501665?

Good guess :)

I have confirmed using mozregression that bug 1501665 is what regressed this.

Keywords: regressionwindow-wanted
Regressed by: 1501665
No longer regressed by: 1536755
Has Regression Range: --- → yes

Given the similarity of the symptoms to bug 1511375, and the diagnosis and fix for that bug, I suspect https://phabricator.services.mozilla.com/D21287.

I don't have a spare tree for a local build right now, but I pushed a backout of https://phabricator.services.mozilla.com/D21287 to build on Try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe24f1202276daebda8ed810a22714534fbc4f10

The bug is indeed fixed in that Try build.

So, I guess the call to UpdateVisualViewportSize in that patch wasn't unnecessary, after all. I think we can't rely on SetResolutionAndScaleTo calling UpdateVisualViewportSize in every case, due to the early exit here.

Assuming that patch isn't needed for a correctness reason, we should probably just back it out.

(In reply to Botond Ballo [:botond] from comment #19)

The bug is indeed fixed in that Try build.

So, I guess the call to UpdateVisualViewportSize in that patch wasn't unnecessary, after all. I think we can't rely on SetResolutionAndScaleTo calling UpdateVisualViewportSize in every case, due to the early exit here.

Assuming that patch isn't needed for a correctness reason, we should probably just back it out.

I agree with this analysis and solution. I'm sorry that I mistakenly claimed that we could early-exit around the call to UpdateVisualViewportSize in that patch. I don't know how to trigger a backout of https://phabricator.services.mozilla.com/D21287. Do I just post a reverting patch as a patch for this bug? I'll assume so unless someone says otherwise.

(In reply to Brad Werth [:bradwerth] from comment #20)

Do I just post a reverting patch as a patch for this bug?

Yep. (You can create the patch using hg backout (or git revert) and then edit its commit message to add in this bug number.)

This change was originally added as a drive-by optimization in Bug 1501665.
The early-exit apparently routes around a needed side effect in the case
where resolution is re-set to its current value.

I'll see if I can craft an effective test as well.

Blocks: 1547101
Attachment #9060823 - Attachment description: Bug 1538681 Part 2: Add a test that scrollbars appear in proper location after resizing viewport. → Bug 1538681 Part 2: Add a disabled test that scrollbars appear in proper location after resizing viewport.
Attachment #9060823 - Attachment description: Bug 1538681 Part 2: Add a disabled test that scrollbars appear in proper location after resizing viewport. → Bug 1538681 Part 2: Add a perma-fail test that scrollbars appear in proper location after resizing viewport.
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f70b79b66592 Part 1: Backout a change that added an early-exit in MobileViewportManager::UpdateResolution. r=botond https://hg.mozilla.org/integration/autoland/rev/9b357ff26675 Part 2: Add a perma-fail test that scrollbars appear in proper location after resizing viewport. r=botond,gl

https://hg.mozilla.org/mozilla-central/rev/f70b79b66592
https://hg.mozilla.org/mozilla-central/rev/9b357ff26675

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox68: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
No longer depends on: 1523844

Hi, the issue from the description is not reproducible. Checked on build Firefox Nightly 68.1a1(2019-07-10) with device Google Pixel C (Android 8.0.0).
I will mark the ticket as Verified.

Status: RESOLVED → VERIFIED
status-firefox68: fixed → verified
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: