727236 - MAPLE: Extra white space at the bottom (scrollable page length is longer than content)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: screenshot of the bottom of the page

1. go to www.cnn.com
2. scroll down towards the bottom
3. keep scrolling

Expected: should not be able to scroll past the bottom of the page
Actual: a large white space after the bottom of the page

Thunderbolt, 2.2.1, Maple 2/14/2012

Comment 1

[Ali Juma [:ajuma]]

I believe this has now been fixed. Are you still seeing this?

Comment 2

[Jeff Muizelaar [:jrmuizel]]

I don't see it. Please reopen if you still do.

Comment 3

[Martijn Wargers (dead)]

I'm still seeing this with a build after the Maple merge, 2012-03-15 Fennec14.0a1.
This is on the HTC Desire HD, while on http://news.google.com

Comment 4

[Martijn Wargers (dead)]

Attachment: Screenshot of the bottom of http://news.google.com

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Simple test case to repro this problem: http://people.mozilla.com/~kgupta/bug/727236.html

Comment 7

[Joe Drew (not getting mail)]

Is this just a dupe of bug 732016?

Comment 8

[Kartikaya Gupta (email:[e-mail])]

(In reply to Joe Drew (:JOEDREW!) from comment #7)
> Is this just a dupe of bug 732016?

No, I think it's a different issue. Either the meta-viewport calculations in browser.js are bad, or the compositor is getting something that's not the actual page size from gecko. I'll look into it.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Fix CSS viewport height calculation

There were a couple of things wrong with the old CSS viewport height expansion calculation (at least the way it was in the latest code). One is that it was using body.clientWidth in getPageZoom() which seems to be 16 pixels short of the actual scrollable width (presumably because of the 8-pixel margins on the html element in the default UA CSS). The other is that the clientWidth was not based on the new CSS viewport width, and so the minScale would be wrong.

I suspect this code always potentially generated the wrong viewport height the first time it was run on a given page, but corrected itself on subsequent runs because on the subsequent runs the clientWidth would be correct from having set the viewport width the previous run. When I added the call to setBrowserSize(kDefault...) in the before-first-paint handler to reset the viewport, that uncovered this error by making it show up at exactly the wrong time.

Comment 13

[Chris Lord [:cwiiis]]

Comment on attachment 606912 [details] [diff] [review]
Fix CSS viewport height calculation

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

Looks good to me.

::: mobile/android/chrome/content/browser.js
@@ +2094,5 @@
> +    // the page is zoomed out to show its full width. Note that before
> +    // we set the viewport width, the "full width" of the page isn't properly
> +    // defined, so that's why we have to call setBrowserSize twice - once
> +    // to set the width, and the second time to figure out the height based
> +    // on the layout at that width.

Maybe this comment should be above the two lines above it, so that those lines aren't mistakenly removed due to overlooking this.

@@ +2096,5 @@
> +    // defined, so that's why we have to call setBrowserSize twice - once
> +    // to set the width, and the second time to figure out the height based
> +    // on the layout at that width.
> +    let minScale = 1.0;
> +    if (this.browser.contentDocument) {

I guess this is missing the comment about it being called before the document is loaded, might want to re-add that here?

Comment 14

[Cristian Nicolae (:xti)]

I am able to see this issue even on About Nightly page on the latest Nightly build.

--
Firefox 14.0a1 (2012-03-19)
Device: Samsung Nexus S
OS: Android 2.3.6

Comment 15

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6da2067a6689

Comment 17

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/mozilla-central/rev/6da2067a6689

Comment 18

[Cristian Nicolae (:xti)]

Verified fixed on:

Firefox 14.0a1 (2012-03-21)
Device: Samsung Nexus S
OS: Android 2.3.6