Summary: | Explore new API that could be used to help build infinitely scrolling websites | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||||||||||||||||||||
Component: | DOM | Assignee: | Beth Dakin <bdakin> | ||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
Severity: | Normal | CC: | bdakin, benjamin, bjonesbe, buildbot, cdumez, cmarcelo, commit-queue, eoconnor, esprehn+autocc, gyuyoung.kim, jonlee, kangil.han, kondapallykalyan, mjs, rniwa, sam, sergio, simon.fraser, syoichi, thorton, tonikitoo | ||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||
Attachments: |
|
Description
Beth Dakin
2014-01-21 14:54:23 PST
Created attachment 221798 [details]
Patch
Here's a patch we can use as a jumping point for further discussion.
Created attachment 221800 [details]
Patch
Created attachment 221804 [details]
Patch
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. (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. Created attachment 221902 [details]
Patch
Names that might be better: webkitwillrevealbottomedge, webkitwillreachbottomsoon, webkitwillreachbottonedgesoon Created attachment 222248 [details]
Patch
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.
Created attachment 222408 [details]
Patch
Here is a patch with a couple more tests, as Sam requested.
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.
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 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
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 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
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 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
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 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
Created attachment 222498 [details]
Patch
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.
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. |