1224307 - Page down scrolls too far on page with fixed position header/footer narrower than the viewport width

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Matt Brubeck (:mbrubeck)]

Bug 780345 added heuristics to detect fixed-position headers and footers, and adjust page scrolling to correct for their height.

These heuristics are currently triggered only if the header or footer's is 100% the width of the scrollport.  This means that it does not work for pages with narrower headers or footers, including popular sites like arstechnica.com and nytimes.com.  This could be fixed by counting headers and footers whose width is greater than some percentage and/or absolute threshold.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This adjusts scrolling for any header whose visible rect is at least 50% of the scrollport width or at least 800px wide.  I chose these numbers more or less arbitrarily.   The absolute threshold is useful for sites like arstechnica.com that have fixed-width headers (so on a very wide-screen display they may be only a small percentage of the viewport width).  The percentage threshold is useful for sites with "liquid" layouts when viewed on narrow screens.

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8686747 [details] [diff] [review]
patch

Seems reasonable. Thanks for the test too.

Comment 3

[Matt Brubeck (:mbrubeck)]

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

Comment 4

[Manu Sridharan]

Sweet!!  If/when there is a build available to try out, I'm definitely willing to test it.

Comment 5

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd0be67d2344a24b6dae7cb91c03138703ca46c6
Bug 1224307 - Fix page scrolling for non-full-width fixed headers/footers [r=tn]

Comment 6

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/aa323766c0f9 for Mulet bustage, https://treeherder.mozilla.org/logviewer.html#?job_id=17207663&repo=mozilla-inbound

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/731ca2fd1aeca2617450eadbe6b79413160b28b4
Bug 1224307 - Fix page scrolling for non-full-width fixed headers/footers [r=tn]

Comment 8

[Manu Sridharan]

I tried to give this a test run on nytimes.com, using this build:

http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-8742dbd030a89d92fb4c64025cc56f38a1223690/try-macosx64-debug/firefox-45.0a1.en-US.mac64.dmg

Hopefully that one includes this patch.  Anyway, for NYTimes, the scrolling behavior seems to be not improved compared to Firefox Beta.  When the window height is ~850 pixels or less, page down scrolling behaves as expected.  But with the window height at ~900 pixels or more, page down scrolling still scrolls too far.  I think the difference is that with a taller window, NYTimes shows an extra header at the top with links to other articles; maybe that isn't being factored in to the calculation.  I will attach screenshots.

Comment 9

[Manu Sridharan]

Attachment: NYTimes without extra toolbar at top; page down scrolling works fine

Comment 10

[Manu Sridharan]

Attachment: NYTimes including extra toolbar at top; page down scrolling still broken

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/731ca2fd1aec

Comment 12

[Matt Brubeck (:mbrubeck)]

> NYTimes including extra toolbar at top; page down scrolling still broken

Could you file a new bug for this please? It looks like some additional fix might be needed for nytimes.com. If someone can make a reduced test case, that would also help.  Thanks!

Comment 13

[Manu Sridharan]

I filed Bug 1225201 for the nytimes.com issue.  Unfortunately I do not have time to try to figure out a reduced test case right now.  Thanks again!