Bug 127371 – Explore new API that could be used to help build infinitely scrolling websites

• Component: DOM
• Status: RESOLVED FIXED

[Beth Dakin]

We want to explore new API that could be used to help build infinitely scrolling websites.

<rdar://problem/14935014>

[Beth Dakin]

Created attachment 221798 [details]
Patch

Here's a patch we can use as a jumping point for further discussion.

[Beth Dakin]

Created attachment 221800 [details]
Patch

[Beth Dakin]

Created attachment 221804 [details]
Patch

[Sam Weinig]

Comment on attachment 221804 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        This patch adds 4 new events that will fire when the user has scrolled close to 
> +        corresponding edge of the document.

This could use some fleshing out, perhaps listing the new event names.

> Source/WebCore/html/HTMLAttributeNames.in:275
> +onwebkitwillrevealbottom
> +onwebkitwillrevealleft
> +onwebkitwillrevealright
> +onwebkitwillrevealtop

I'm not sure we need to support these as attributes, but Ted should weigh in.

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

Nit: Top and bottom aren't really directions, maybe, "edge"?

> Source/WebCore/page/FrameView.cpp:2964
> +            frame().document()->dispatchWindowEvent(willRevealEvent.release(), frame().document()->domWindow());

We should probably fire these asynchronously, which can be done via frame().document()->enqueueWindowEvent(event.release()); We may also want null check document() here.

[Beth Dakin]

(In reply to comment #4)
> (From update of attachment 221804 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221804&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        This patch adds 4 new events that will fire when the user has scrolled close to 
> > +        corresponding edge of the document.
> 
> This could use some fleshing out, perhaps listing the new event names.
> 

Sure. I added a little more information.

> > Source/WebCore/html/HTMLAttributeNames.in:275
> > +onwebkitwillrevealbottom
> > +onwebkitwillrevealleft
> > +onwebkitwillrevealright
> > +onwebkitwillrevealtop
> 
> I'm not sure we need to support these as attributes, but Ted should weigh in.
> 

Ted and I spoke in person. He thinks we should support them. He thinks that not including them would introduce a confusing inconsistency even though they are kind of silly.

> > Source/WebCore/page/FrameView.cpp:2949
> > +    // For each direction (top, bottom, left and right), send the will reveal edge event for that direction
> 
> Nit: Top and bottom aren't really directions, maybe, "edge"?
> 

Fixed.

> > Source/WebCore/page/FrameView.cpp:2964
> > +            frame().document()->dispatchWindowEvent(willRevealEvent.release(), frame().document()->domWindow());
> 
> We should probably fire these asynchronously, which can be done via frame().document()->enqueueWindowEvent(event.release()); We may also want null check document() here.

Fixed.

Thanks for the feedback! I will upload a new patch shortly. I was spreading a rumor earlier that this patch has a problem where it doesn't always fire the event if you scroll too fast. Turns out that was a bug in my test app NOT in this implementation.

[Beth Dakin]

Created attachment 221902 [details]
Patch

[Beth Dakin]

Names that might be better: webkitwillrevealbottomedge, webkitwillreachbottomsoon, webkitwillreachbottonedgesoon

[Beth Dakin]

Created attachment 222248 [details]
Patch

[WebKit Commit Bot]

Attachment 222248 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/EventNames.h:123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/EventNames.h:128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Beth Dakin]

Created attachment 222408 [details]
Patch

Here is a patch with a couple more tests, as Sam requested.

[WebKit Commit Bot]

Attachment 222408 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/EventNames.h:123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/EventNames.h:128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 222408 [details]
Patch

Attachment 222408 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5055793170743296

New failing tests:
fast/events/will-reveal-edges-window-attributes.html

[Build Bot]

Created attachment 222415 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222408 [details]
Patch

Attachment 222408 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6034654066900992

New failing tests:
fast/events/will-reveal-edges-window-attributes.html

[Build Bot]

Created attachment 222417 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222408 [details]
Patch

Attachment 222408 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5638829880901632

New failing tests:
fast/events/will-reveal-edges-window-attributes.html

[Build Bot]

Created attachment 222422 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 222408 [details]
Patch

Attachment 222408 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5101886290001920

New failing tests:
fast/events/will-reveal-edges-window-attributes.html

[Build Bot]

Created attachment 222428 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Beth Dakin]

Created attachment 222498 [details]
Patch

[WebKit Commit Bot]

Attachment 222498 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/EventNames.h:123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/EventNames.h:128:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Beth Dakin]

Thanks Sam! I'll wait a day or two to land in case anyone has more feedback. And I filed https://bugs.webkit.org/show_bug.cgi?id=127810 to make this work for overflow:scroll.

[Beth Dakin]

http://trac.webkit.org/changeset/163092

[Beth Dakin]

http://trac.webkit.org/changeset/163092