Bug 34371 - REGRESSION (r53718): When scrolling a tall window by page, the overlap between pages is too big
: REGRESSION (r53718): When scrolling a tall window by page, the overlap betwee...
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: InRadar, Regression
:
:
  Show dependency treegraph
 
Reported: 2010-01-30 00:27 PST by
Modified: 2012-11-30 12:31 PST (History)


Attachments
Cap the overlap amount at 40 pixels (9.22 KB, patch)
2010-01-30 00:30 PST, mitz@webkit.org
no flags Review Patch | Details | Formatted Diff | Diff
Allow ScrollbarTheme to cap the overlap between pages, and set a cap of in ScrollbarThemeMac (17.70 KB, patch)
2010-01-30 01:31 PST, mitz@webkit.org
simon.fraser: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-01-30 00:27:41 PST
<http://trac.webkit.org/changeset/53718> changed scroll by page behavior to make the overlap between pages a fraction of the visible height (width when scrolling horizontally). This was motivated by the fixed-size overlap being too big for small areas. However, it resulted in the overlap being too big for big areas. For example, for a visible height of around 1000 pixels, the overlap is 125 pixels, compared to 40 pixels before the change. In Firefox on Mac OS X, the overlap for a similar visible height is only 32 pixels.
------- Comment #1 From 2010-01-30 00:30:39 PST -------
No, the original motivation was the overlap being too small for big areas, making page scrolls hinder readability.  The change was made to match IE which behaves much more nicely than Fx: having a 125 px overlap at 1000 px is exactly what we wanted.

Fixing the excessive overlap in tiny areas was an added bonus, but not the main goal.

FWIW, hyatt's comment in IRC was "oh, my original 40 px change was an attempt to match IE, so if they do 1/8th page, let's do that".
------- Comment #2 From 2010-01-30 00:30:53 PST -------
Created an attachment (id=47758) [details]
Cap the overlap amount at 40 pixels
------- Comment #3 From 2010-01-30 00:32:12 PST -------
(From the original bug:

"This means that when we scroll by "one page", we overlap by 40 px.  This is an
extremely small amount -- for any nontrivial window height, barely enough to be
noticeable.  ...

By contrast, IE overlaps by 1/8th of the page height.  This strikes me as much
better for readability (and indeed, when reading on my full-height window, it's
much easier to read without losing my place).")
------- Comment #4 From 2010-01-30 00:34:42 PST -------
I am not aware of any application on Mac OS X where the overlap is anywhere near 125 pixels for a window size of 1000 pixels. This makes Safari look broken on Mac OS X.
------- Comment #5 From 2010-01-30 00:37:26 PST -------
Well we certainly don't want to cap at 40 px, that was exactly the problem with the old behavior.  And Chromium on OS X (and all other OSes, of course) wants to keep the current behavior.

This is why I asked about platform-specific behavior and objections on the old bug.

Perhaps we can put in a Safari/Mac-specific override.
------- Comment #6 From 2010-01-30 00:39:15 PST -------
Attachment 47758 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/220331
------- Comment #7 From 2010-01-30 00:40:11 PST -------
(Though I wonder if everyone would agree that it "looks broken".  It doesn't
look broken to me on my MacBook, or on my 1920-pixel-high fullscreen-browsing
Windows desktop either.)
------- Comment #8 From 2010-01-30 01:31:07 PST -------
Created an attachment (id=47759) [details]
Allow ScrollbarTheme to cap the overlap between pages, and set a cap of in ScrollbarThemeMac
------- Comment #9 From 2010-01-30 01:34:15 PST -------
I am happy with this patch in principle.
------- Comment #10 From 2010-01-30 02:00:21 PST -------
Attachment 47759 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/221255
------- Comment #11 From 2010-02-01 15:57:12 PST -------
This will break Gtk:
../../WebCore/platform/gtk/WheelEventGtk.cpp: In constructor ‘WebCore::PlatformWheelEvent::PlatformWheelEvent(GdkEventScroll*)’:
../../WebCore/platform/gtk/WheelEventGtk.cpp:72: error: ‘cScrollbarPixelsPerLineStep’ was not declared in this scope
------- Comment #12 From 2010-02-03 22:05:05 PST -------
<rdar://problem/7611154>
------- Comment #13 From 2010-02-04 08:12:07 PST -------
(From update of attachment 47759 [details])
> Index: WebCore/platform/Scrollbar.h
> ===================================================================

> +    static int pixelsPerLineStep() { return 40; }
> +    static float minFractionToStepWhenPaging() { return 0.875f; }
> +    static int maxAmountToKeepWhenPaging();

I think I'd like to see "pixels" in the name of this method.I'm not super-keen on "amount to keep" either, but can't think of a better term.
------- Comment #14 From 2010-02-04 08:53:28 PST -------
Maybe "overlap" for "amountToKeep"?
------- Comment #15 From 2010-02-04 09:37:02 PST -------
<http://trac.webkit.org/projects/webkit/changeset/54345>.
------- Comment #17 From 2012-03-12 13:48:21 PST -------
Rebaselined in http://trac.webkit.org/changeset/110472. Please let me know if you think this rebaseline is incorrect.
------- Comment #18 From 2012-03-12 14:13:23 PST -------
This is a side effect of running the tests with ScrollbarThemeMock.
------- Comment #19 From 2012-03-12 14:14:21 PST -------
We’ve lost test coverage for all the code in ScrollbarThemeMac :(
------- Comment #20 From 2012-11-30 12:24:13 PST -------
Could this be user-configurable, a user asks. See https://forum.kde.org/viewtopic.php?f=15&t=108930&sid=c585b1db933e0c0be00ccd27a67e3638
------- Comment #21 From 2012-11-30 12:31:34 PST -------
(In reply to comment #20)
> Could this be user-configurable, a user asks. See https://forum.kde.org/viewtopic.php?f=15&t=108930&sid=c585b1db933e0c0be00ccd27a67e3638

That'd be up to the individual port in question.  The patch that was landed here allows ports to control the overlap amount, so it's up to those ports to decide what they want to do.