The programmatic scroll would fire scrollend events.
Created attachment 393744 [details] scrollend-programmatic-for-ews
Created attachment 393745 [details] scrollend-programmatic-for-review
Created attachment 393746 [details] scrollend-programmatic-for-ews
Created attachment 393747 [details] scrollend-programmatic-for-ews
Comment on attachment 393745 [details] scrollend-programmatic-for-review View in context: https://bugs.webkit.org/attachment.cgi?id=393745&action=review > Source/WebCore/dom/Document.cpp:804 > + m_pendingScrollendEventTargetList = nullptr; Should everything in this patch be "ScrollEnd"? Did you check what we do for other events? > Source/WebCore/page/FrameView.cpp:2286 > +void FrameView::setScrollPosition(const ScrollPosition& scrollPosition, ScrollClamping clamping, bool animated, bool saveScrollendPosition) I believe Simon complained in the past about "boolean trap". I wonder whether we want enums here? > Source/WebCore/platform/ScrollView.cpp:543 > + if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation && newScrollPosition == this->scrollPosition()) { this line is very long, maybe consider https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op ? > Source/WebCore/platform/ScrollView.cpp:-534 > - scrollAnimator().cancelAnimations(); Can you explain this behavior change? It seems you move it above where the if is no longer done. > Source/WebCore/rendering/RenderLayer.cpp:2848 > + bool autoscrollNotInProgress = !renderer().frame().eventHandler().autoscrollInProgress(); It seems the autscrollstatus change can be done in a separate preliminary patch?
Created attachment 395799 [details] scrollend-programmatic-for-ews
Created attachment 395800 [details] scrollend-programmatic-for-review
Created attachment 395801 [details] scrollend-programmatic-for-ews
Created attachment 395802 [details] scrollend-programmatic-for-review
Created attachment 395805 [details] scrollend-programmatic-for-ews
Created attachment 395806 [details] scrollend-programmatic-for-review
Hi Fred, Thanks for the review:) > > > Source/WebCore/dom/Document.cpp:804 > > + m_pendingScrollendEventTargetList = nullptr; > > Should everything in this patch be "ScrollEnd"? Did you check what we do for > other events? > Done. > > Source/WebCore/page/FrameView.cpp:2286 > > +void FrameView::setScrollPosition(const ScrollPosition& scrollPosition, ScrollClamping clamping, bool animated, bool saveScrollendPosition) > > I believe Simon complained in the past about "boolean trap". I wonder > whether we want enums here? > Done. > > Source/WebCore/platform/ScrollView.cpp:543 > > + if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation && newScrollPosition == this->scrollPosition()) { > > this line is very long, maybe consider > https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op ? > Done. > > Source/WebCore/platform/ScrollView.cpp:-534 > > - scrollAnimator().cancelAnimations(); > > Can you explain this behavior change? It seems you move it above where the > if is no longer done. > We should cancel the animation before return. There's a tiny chance that animation doesn't stop. Moved this change to https://bugs.webkit.org/show_bug.cgi?id=210171. > > Source/WebCore/rendering/RenderLayer.cpp:2848 > > + bool autoscrollNotInProgress = !renderer().frame().eventHandler().autoscrollInProgress(); > > It seems the autscrollstatus change can be done in a separate preliminary > patch? Done, I move this change to https://bugs.webkit.org/show_bug.cgi?id=210171
Created attachment 395939 [details] scrollend-programmatic-for-ews
Created attachment 395940 [details] scrollend-programmatic-for-review
Created attachment 395943 [details] scrollend-programmatic-for-ews
Created attachment 395944 [details] scrollend-programmatic-for-review
Comment on attachment 395944 [details] scrollend-programmatic-for-review Hi, The patch is ready to review\o/ Thanks:) This is based on the parsing patch(208802) and code style fix up for scroll animated patch(bug 210171).
Created attachment 396058 [details] scrollend-programmatic-for-ews
Created attachment 396059 [details] scrollend-programmatic-for-review
Created attachment 396062 [details] scrollend-programmatic-for-review
Comment on attachment 396062 [details] scrollend-programmatic-for-review Hi, Rebase the code. After commit the code style fix up patch and test import patch, now this patch is only based on the parsing patch(bug 208802). I wonder if I can commit the parsing patch now?
Created attachment 399465 [details] scrollend-programmatic-for-ews
Created attachment 399466 [details] scrollend-programmatic-for-review
Chrome has shipped the new "scrollend" event as an experimental feature. Are there plans to enable this feature to Safari as well?
Is there a spec for this new event?
(In reply to Simon Fraser (smfr) from comment #25) > Is there a spec for this new event? Yes, here you are: https://wicg.github.io/overscroll-scrollend-events/ Chrome has shipped it behind experimental flag (for now, I think they only implemented the scrollend event).
The scrollend event shipped in Chrome 114 last month. Simple test page (the page will turn yellow after the scroll): https://output.jsbin.com/ludiban/quiet The event seems to now be defined in the CSSOM View spec: https://drafts.csswg.org/cssom-view/#eventdef-document-scrollend
Firefox also shipped 'scrollend'. Please also enable it in Safari on both iOS and macOS. Would make some components much easier than with debouncing and heuristics to detect scroll end. Thanks.
Please implement the 'scrollend' event, because we must use a polyfill for Safari only. Thanks!