Closed Bug 738641 Opened 10 years ago Closed 9 years ago

Viewport jumps to the top of the page when reloaded in the landscape mode on espn.com

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox14 verified, firefox15 verified, blocking-fennec1.0 soft)

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- soft

People

(Reporter: paul.feher, Assigned: kats)

Details

(Whiteboard: [viewport])

Attachments

(2 files)

Nightly Fennec 14.0a1 (2012-03-23)
Device: Samsung Nexus S
OS: Android 2.3.6

Steps to reproduce:
1) Load espn.com in landscape mode
2) Scroll page down
3) Reload the page

Expected:
The page viewport stays the same.

Actual:
Viewport jumps to the top of the page.

NOTE:In portrait mode this issue is not reproducible.
When I reload the page in landscape, it starts off showing the top but by the time the page is done loading it has jumped to the right scroll position. That's pretty much what I would expect.
Attached file Log file
This issue is reproducible every time on gsp.ro. For espn.com is reproducible only sometimes.
Paul, when you hit the reload button, has the throbber stopped?  If you don't recall, can you check to see what happens when the throbber stops?
Both the sites request no-cache. I think this is invalid, no-cache forces Firefox to refetch to the files.

$ wget --spider -S m.espn.go.com
Spider mode enabled. Check if remote file exists.
--2012-03-26 09:17:01--  http://m.espn.go.com/
Resolving m.espn.go.com... 68.71.216.184
Connecting to m.espn.go.com|68.71.216.184|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.0 302 Found
  Location: /wireless/
  Server: BigIP
  Connection: Keep-Alive
  Content-Length: 0
Location: /wireless/ [following]
Spider mode enabled. Check if remote file exists.
--2012-03-26 09:17:01--  http://m.espn.go.com/wireless/
Connecting to m.espn.go.com|68.71.216.184|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.0 302 Moved Temporarily
  Connection: Keep-Alive
  Content-Length: 143
  Content-Type: text/html; charset=iso-8859-1
  Location: /wireless/?wjb=
  Server: barista/3.3.6
Location: /wireless/?wjb= [following]
Spider mode enabled. Check if remote file exists.
--2012-03-26 09:17:01--  http://m.espn.go.com/wireless/?wjb=
Connecting to m.espn.go.com|68.71.216.184|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.0 200 OK
  Cache-Control: no-cache, must-revalidate
  Connection: Keep-Alive
  Content-Length: 18363
  Content-Type: text/html; charset=iso-8859-1
  Expires: Mon, 26 Mar 2012 16:17:01 GMT
  Last-Modified: Mon, 26 Mar 2012 16:17:01 GMT
  Pragma: no-cache
  Server: barista/3.3.6
  Set-Cookie: interstitial_test=true; expires=Tue, 27-Mar-2012 16:17:01 GMT; path=/; domain=m.espn.go.com;
  Set-Cookie: dimg-vid=h09pz9wo576546933; expires=Sat, 25-Mar-2017 16:17:01 GMT; path=/; domain=.m.espn.go.com;
Length: 18363 (18K) [text/html]
Remote file exists and could contain further links,
but recursion is disabled -- not retrieving.


$ wget --spider -S gsp.ro
Spider mode enabled. Check if remote file exists.
--2012-03-26 09:17:35--  http://gsp.ro/
Resolving gsp.ro... 89.47.247.39, 89.47.247.38
Connecting to gsp.ro|89.47.247.39|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 302 Moved Temporarily
  Server: nginx
  Date: Mon, 26 Mar 2012 16:17:36 GMT
  Content-Type: text/html
  Connection: keep-alive
  Keep-Alive: timeout=20
  Set-Cookie: PHPSESSID=49d9q314t1up3tou1e75vebfh5; path=/
  Expires: Thu, 19 Nov 1981 08:52:00 GMT
  Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
  Pragma: no-cache
  Location: http://www.gsp.ro/
  X-Whom: Digital HTTPD v3.6
Location: http://www.gsp.ro/ [following]
Spider mode enabled. Check if remote file exists.
--2012-03-26 09:17:36--  http://www.gsp.ro/
Resolving www.gsp.ro... 89.47.247.39, 89.47.247.38
Connecting to www.gsp.ro|89.47.247.39|:80... connected.
HTTP request sent, awaiting response... 
  HTTP/1.1 200 OK
  Server: nginx
  Date: Mon, 26 Mar 2012 16:17:38 GMT
  Content-Type: text/html
  Connection: keep-alive
  Keep-Alive: timeout=20
  Set-Cookie: PHPSESSID=65q7d0drk1i5prvn9gchf82l60; path=/
  Expires: Thu, 19 Nov 1981 08:52:00 GMT
  Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
  Pragma: no-cache
  X-Whom: Digital HTTPD v3.6
Length: unspecified [text/html]
Remote file exists and could contain further links,
but recursion is disabled -- not retrieving.
I was able to reproduce this issue on espn.com while the device was in landscape mode. 

(In reply to Naoki Hirata :nhirata from comment #4)
> Paul, when you hit the reload button, has the throbber stopped?  If you
> don't recall, can you check to see what happens when the throbber stops?

I tried this, and the throbber will stop after the page is fully loaded.
I was able to reproduce this twice, just after switching landscape/portrait. I could reproduce this in portrait and in landscape mode.
This was on the Samsung Galaxy Nexus, Android 4.0.2.
I think no-cache behaviour is interesting, but not related to this bug. It shouldn't affect the scroll position Gecko has stored for the page. In the log that Paul attached, I see this which I think is the problematic part.

10:08:58.933: D/GeckoPanZoomController(798): generating valid viewport using v=RectF(0.0, 1022.0, 800.0, 1392.0005) p=(800.0,370.0) z=1.5000018
03-26 10:08:58.933: D/GeckoPanZoomController(798): generated valid viewport as v=RectF(5.187988E-4, 0.0, 800.0005, 370.0005) p=(800.00104,370.0005) z=1.5000038

What's happening here is that Java is getting told that it should scroll to y=1022 (I assume this is the restoring of the scroll position) but at that point the page is only 370.0 pixels tall, so it refuses to scroll down that far. Either Gecko is trying to restore the scroll position before the page size is updated, or (more likely) we're sending the wrong page size from browser.js to Java when we request the scroll. I know the page size calculation in the tab.getViewport() in browser.js wasn't updated in maple, and may be wrong now.
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → beta+
Whiteboard: [viewport]
blocking-fennec1.0: beta+ → +
Assignee: nobody → bugmail.mozilla
Issue is also reproducible on http://jquery.malsup.com/form/#fields using Nightly/14.0a1 2012-04-06 on a Motorola Droid 2 (Android 2.3). The page is not loaded all the way to the top but it is positioned half a screen lower when reloaded.
blocking-fennec1.0: + → soft
(In reply to adrian tamas from comment #9)
> Issue is also reproducible on http://jquery.malsup.com/form/#fields using
> Nightly/14.0a1 2012-04-06 on a Motorola Droid 2 (Android 2.3). The page is
> not loaded all the way to the top but it is positioned half a screen lower
> when reloaded.

I'm not able to reproduce this, exactly. However I do see an issue where if I scroll to the bottom of http://jquery.malsup.com/form/#fields and then reload, it doesn't reload to the same spot but is positioned halfway down the page. The reason this happens is that the page is changing size dynamically, and at some point during the loading process the page height shrinks to 776 pixels which causes us to scroll upwards in order to not display overscroll. Then the page re-expands and we remain at the same position. The reason the page srhinks/re-expands is because it implements the "tabbed" behaviour by showing/hiding divs.

I'm still unable to reproduce the original bug on espn.com and gsp.ro.
Ah, I'm able to reproduce the problem using an HTC something-or-other. I'll debug it further.
Rounding errors were causing the numbers to be off a little. On espn.com in landscape (zoom 1.5), I was getting gScreenWidth=800 and cssPageWidth=533, so pageWidth=533 * 1.5=799.5, which is less than gScreenWidth and was causing the check to fail.
Attachment #619627 - Flags: review?(chrislord.net)
Comment on attachment 619627 [details] [diff] [review]
Blasted rounding errors!

Review of attachment 619627 [details] [diff] [review]:
-----------------------------------------------------------------

Looks alright to me.

::: mobile/android/chrome/content/browser.js
@@ +1925,5 @@
>         * this causes the page size to jump around wildly during page load. After the page is loaded,
>         * send updates regardless of page size; we'll zoom to fit the content as needed.
> +       *
> +       * Also, we need to compare the page size returned from getPageSize (in CSS pixels) to the floored
> +       * screen size in CSS pixels because the page size returned from getPageSize may be also be floored.

s/may be also be/may also be/
Attachment #619627 - Flags: review?(chrislord.net) → review+
Comment on attachment 619627 [details] [diff] [review]
Blasted rounding errors!

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: sometimes the page size might not be reported properly, resulting in miscellaneous scrolling weirdness
Testing completed (on m-c, etc.): locally. this patch hasn't made it to m-c yet because of tree closure
Risk to taking this patch (and alternatives if risky): relatively low risk. if the condition is wrong it will might just cause a little jump during page load but nothing major
String changes made by this patch: none
Attachment #619627 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a2a7e9f92fbe
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #619627 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fix on:
Nightly 15.0a1 (2012-05-21)
Nightly 14.0a2 (2012-05-21)
Device: HTC Desire Z
OS: Android 2.3.3
Status: RESOLVED → 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.