1525948 - Scroll position persists after clicking on a link

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[csheany]

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

Steps to reproduce:

1. Open https://github.com/mozilla-mobile/focus-android
2. Request desktop site
3. Tap Commits
4. Scroll to the bottom
5. Tap Older

Actual results:

The bottom of the page is in view

Expected results:

The top of the page is in view

Comment 1

[Botond Ballo [:botond]]

Regression compared to Firefox 65.

Comment 2

[Botond Ballo [:botond]]

Mozregression points to bug 1520077:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fd129c882b775e821dea55747d35fc232f58b888&tochange=1d547e5a7a51b68a98a9696c52e5fbe4888bdb7b

It's not immediately clear how that change could cause this.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I can't reproduce at all on my Android phone.

Comment 4

[Botond Ballo [:botond]]

I was testing on a tablet. I just tried on a phone, and I can't repro there either. Guess it's tablet-specific.

Comment 5

[Andrei Bodea[:andrei]]

I couldn't reproduce this issue on the latest Nightly build 67.0a1 with a Google Pixel 3 XL (Android P) and Sony Xperia Z5(Android 7.0).
Every time when I tapped on the older the top page was in view. I tried to reproduce this while the web page was zoomed in but with not so much success.
Can you please mention the exact device, android version and firefox version that are you using while reproducing this issue.

Comment 6

[csheany]

Thank you for looking into it. The device is a Samsung Galaxy Tab A.

I am using the latest Nightly.

As Botond mentioned, only tablets might be affected.

Comment 7

[miralobontiu]

Hi,
I was able to reproduce the issue with a Samsung Galaxy Tab S3, on Firefox 65.0.1, and Beta 66.0b7.
But I was not able to reproduce it on the latest Nightly 67.0a1 (2019-02-12).
I will confirm the issue.
Thanks for the report!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I'm confused on how to rectify comment 2 with comment 7. Can someone please try to explain?

Comment 9

[Ioana Chiorean]

Mira, can you help with a regression?

Comment 10

[Botond Ballo [:botond]]

I think comment 7 may be confusing the "good" and "bad" outcomes.

The scroll position being at the top of the new page, which happens in 65 and 66, is the "good" outcome. The scroll position being near the bottom of the new page, which happens in nightly, is the "bad" outcome.

It's a bit confusing because if navigation does not occur, you expect the scroll position to be preserved. If navigation does occur (as in these STR), you expect the scroll position to be at the beginning of the new page.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

I'll go with that, thanks :)

Comment 12

[Hiroyuki Ikezoe (:hiro)]

I will see what's going on there.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

I suppose we still want to fix this in 67.

Anyway, I thought I can reproduce the issue on RDM with specifying 'Nexus 10' there (and enabling apz.allow_zooming pref). But the issue I can see is also reproducible without disabling the minimum scale change. So the issue I am seeing might be a different issue.

I also tried on Android emulator with tablet mode (I believe it's tablet mode, I am using 1280x800 resolution), but I can't reproduce it yet.

Comment 14

[miralobontiu]

Thank you Botond, I was misguided by the "good" and "bad" outcome.
I rectify Comment 7, the issue is reproducible only in Nightly 67.0a1.
Thank you, Ryan, for changing the flags.
I`m sorry for the confusion.

Comment 15

[Botond Ballo [:botond]]

Let's park this bug with me as I'm able to reproduce it. I will investigate when I get a chance.

Comment 16

[Botond Ballo [:botond]]

The issue here is related to the layout viewport being larger than it should be on this page. The layout viewport size should match the visual viewport size when the page is at its minimum zoom, but on this page it's taller, tall enough to include the entire content size.

Comment 17

[Botond Ballo [:botond]]

I have a fix for this locally. I would like to write a test as well.

Comment 18

[Botond Ballo [:botond]]

Attachment: Bug 1525948 - Add Min() and Max() functions for ScaleFactor. r=kats

Comment 19

[Botond Ballo [:botond]]

Attachment: Bug 1525948 - Ensure the layout viewport retains the aspect ratio of the visual viewport. r=hiro

Depends on D19995

Comment 20

[Botond Ballo [:botond]]

Attachment: Bug 1525948 - Add a reftest. r=hiro

Depends on D19996

Comment 21

[Botond Ballo [:botond]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=aa1082aa29961aa92643a094dc0c67230df3acd9

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d3ca8862f6ff
https://hg.mozilla.org/mozilla-central/rev/bf8ca7933ead

Comment 23

[Sorina Florean [:sflorean]]

Verified as fixed on latest Nightly build (67.0a1) with Samsung Galaxy Tab S3 (affected device).