Bug 63678 - More ScrollableArea scaffolding
Summary: More ScrollableArea scaffolding
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-29 18:19 PDT by Anders Carlsson
Modified: 2011-06-30 12:40 PDT (History)
3 users (show)

See Also:

Attachments
Patch (9.34 KB, patch)
2011-06-29 18:21 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (10.69 KB, patch)
2011-06-30 10:52 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (12.60 KB, patch)
2011-06-30 11:04 PDT, Anders Carlsson 		no flags Details | Formatted Diff | Diff
Patch (13.47 KB, patch)
2011-06-30 12:02 PDT, Anders Carlsson 		mitz: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2011-06-29 18:19:12 PDT 
More ScrollableArea scaffolding
Comment 1 Anders Carlsson 2011-06-29 18:21:23 PDT 
Created attachment 99198 [details]
Patch
Comment 2 WebKit Review Bot 2011-06-29 18:27:26 PDT 
Comment on attachment 99198 [details]
Patch

Attachment 99198 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8962275
Comment 3 Sam Weinig 2011-06-29 18:29:30 PDT 
Comment on attachment 99198 [details]
Patch

Fresh.  (/me is totally not reviewing this while on vacation.)  I think some other ports may have other classes that derive from ScrollView, so you will probably have to patch them too (or just add a default implementation to ScrollView.)
Comment 4 Anders Carlsson 2011-06-30 10:52:20 PDT 
Created attachment 99333 [details]
Patch
Comment 5 WebKit Review Bot 2011-06-30 11:01:37 PDT 
Comment on attachment 99333 [details]
Patch

Attachment 99333 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8962544
Comment 6 Anders Carlsson 2011-06-30 11:04:58 PDT 
Created attachment 99336 [details]
Patch
Comment 7 Anders Carlsson 2011-06-30 12:02:45 PDT 
Created attachment 99352 [details]
Patch
Comment 8 mitz 2011-06-30 12:13:20 PDT 
Comment on attachment 99352 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99352&action=review

> Source/WebCore/platform/ScrollableArea.cpp:208
> +bool ScrollableArea::isPinnedInDirectionOfScrollDelta(const IntSize& scrollDelta) const

You can drop “OfScrollDelta” from the name.

> Source/WebCore/platform/chromium/FramelessScrollView.cpp:62
> +    // FIXME

Can you elaborate?

> Source/WebCore/rendering/RenderLayer.cpp:797
> +    RenderObject* nextRenderer = renderer()->parent();
> +
> +    while (nextRenderer) {
> +        if (nextRenderer->isBox() && toRenderBox(nextRenderer)->canBeScrolledAndHasScrollableArea())
> +            return nextRenderer->enclosingLayer();
> +
> +        nextRenderer = nextRenderer->parent();
> +    }
> +
> +    return 0;

Looks like a for() loop.

> Source/WebCore/rendering/RenderLayer.cpp:2002
> +    // FIXME: We should return the frame view here (or possibly a parent frame view,

s/parent/ancestor/?

> Source/WebKit/chromium/src/WebScrollbarImpl.cpp:295
> +    // FIXME

More detail?
Comment 9 Anders Carlsson 2011-06-30 12:40:59 PDT 
Committed r90152: <http://trac.webkit.org/changeset/90152>