Bug 131071 - willReveal edge events should be hooked up for overflow:scroll
Summary: willReveal edge events should be hooked up for overflow:scroll
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-01 15:30 PDT by Beth Dakin
Modified: 2014-04-01 21:50 PDT (History)
9 users (show)

See Also:

Attachments
Patch (15.23 KB, patch)
2014-04-01 15:38 PDT, Beth Dakin 		no flags Details | Formatted Diff | Diff
Patch (15.32 KB, patch)
2014-04-01 15:45 PDT, Beth Dakin 		sam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 Beth Dakin 2014-04-01 15:30:37 PDT 
Our new willRevealBottom/Top/Left/Right events should be hooked up for overflow:scroll.

<rdar://problem/16190392>
Comment 1 Beth Dakin 2014-04-01 15:38:22 PDT 
Created attachment 228329 [details]
Patch
Comment 2 WebKit Commit Bot 2014-04-01 15:39:26 PDT 
Attachment 228329 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/Document.cpp:5623:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Beth Dakin 2014-04-01 15:45:17 PDT 
Created attachment 228330 [details]
Patch
Comment 4 WebKit Commit Bot 2014-04-01 15:46:20 PDT 
Attachment 228330 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/Document.cpp:5623:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Sam Weinig 2014-04-01 20:12:42 PDT 
Comment on attachment 228330 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:5618
> +     // For each edge (top, bottom, left and right), send the will reveal edge event for that direction

You have an extra space here.

> Source/WebCore/dom/Document.h:1177
> +    void sendWillRevealEdgeEventsIfNeeded(const IntPoint& oldPosition, const IntPoint& newPosition, const IntRect& visibleRect, const IntSize& contentsSize, Element* target = 0);

You should use Element* target = nullptr.  I would also rename this to enqueueWillRevealEdgeEventsIfNeeded.  This might also be better on the DocumentEventQueue class, not sure.
Comment 6 Beth Dakin 2014-04-01 21:50:33 PDT 
(In reply to comment #5)
> (From update of attachment 228330 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228330&action=review
> 
> > Source/WebCore/dom/Document.cpp:5618
> > +     // For each edge (top, bottom, left and right), send the will reveal edge event for that direction
> 
> You have an extra space here.
> 
> > Source/WebCore/dom/Document.h:1177
> > +    void sendWillRevealEdgeEventsIfNeeded(const IntPoint& oldPosition, const IntPoint& newPosition, const IntRect& visibleRect, const IntSize& contentsSize, Element* target = 0);
> 
> You should use Element* target = nullptr.  I would also rename this to enqueueWillRevealEdgeEventsIfNeeded.  This might also be better on the DocumentEventQueue class, not sure.

Fixed both. Thanks, Sam! http://trac.webkit.org/changeset/166630