On Wed, Apr 12, 2023 at 8:34 AM Alex Russell <slightly...@chromium.org> wrote:
> Regardless of the LGTM's, it's not sufficient to cite a CSS WG draft. > Please advise the TAG with an FYI before you ship. > Hi Alex, In this case I think a TAG review is not necessary for the reason <https://www.chromium.org/blink/guidelines/api-owners/process-exceptions/> that it's a WG-agreed spec already shipping at least one other browser. > > On Wednesday, April 12, 2023 at 8:20:44 AM UTC-7 Daniel Bratell wrote: > >> LGTM3 >> >> /Daniel >> On 2023-04-12 10:57, Yoav Weiss wrote: >> >> Thanks for verifying this and for filing Firefox issues. >> >> LGTM2 >> >> On Tue, Apr 11, 2023 at 10:46 PM David Awogbemila < >> awogbem...@chromium.org> wrote: >> >>> >>> >>> On Fri, Apr 7, 2023 at 4:49 AM Yoav Weiss <yoavwe...@chromium.org> >>> wrote: >>> >>>> >>>> >>>> On Thu, Apr 6, 2023 at 2:39 AM David Awogbemila < >>>> awogbem...@chromium.org> wrote: >>>> >>>>> >>>>> >>>>> On Wed, Apr 5, 2023 at 12:11 AM Yoav Weiss <yoavwe...@chromium.org> >>>>> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Fri, Mar 31, 2023 at 3:50 PM David Awogbemila < >>>>>> awogbem...@chromium.org> wrote: >>>>>> >>>>>>> Contact emails >>>>>>> arg...@google.com, awogbem...@google.com >>>>>>> >>>>>>> Explainer >>>>>>> https://github.com/argyleink/scrollend-explainer/blob/main/README.md >>>>>>> >>>>>> >>>>>> The explainer says this shipped in 114. I guess it should say y'all >>>>>> are expecting to ship at that point :) >>>>>> >>>>>> >>>>>>> >>>>>>> Specification https://drafts.csswg.org/cssom-view/#scrolling-events >>>>>>> >>>>>>> Summary >>>>>>> >>>>>>> Scrollend events help developers reliably tell when a scroll has >>>>>>> completed (including both the scroll itself and any updates to offsets >>>>>>> from >>>>>>> the scroll). Knowing when a scroll has completed is useful for various >>>>>>> reasons, e.g. synchronizing some logic on the snapped section, fetching >>>>>>> stuff in a list, triggering new animations, etc. This feature greatly >>>>>>> simplifies the logic for handling end-of-scroll effects, ensuring that >>>>>>> they >>>>>>> are consistent across many different input modalities. Currently, >>>>>>> developers address this need by observing scroll events and building >>>>>>> ad-hoc >>>>>>> timeout algorithms. >>>>>>> >>>>>>> >>>>>>> Blink component Blink>Scroll >>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EScroll> >>>>>>> >>>>>>> Search tags scroll <https://chromestatus.com/features#tags:scroll> >>>>>>> >>>>>>> TAG review >>>>>>> >>>>>>> TAG review status Not applicable >>>>>>> >>>>>> >>>>>> Agree this is not needed, as this is following WG agreed-upon >>>>>> behavior, that has already shipped in one implementation. >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> Risks >>>>>>> >>>>>>> >>>>>>> Interoperability and Compatibility >>>>>>> >>>>>>> >>>>>>> *Gecko*: Shipped/Shipping ( >>>>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1797013) >>>>>>> >>>>>>> *WebKit*: No signal ( >>>>>>> https://github.com/WebKit/standards-positions/issues/150) There >>>>>>> hasn't been an explicit position attached to the position request yet >>>>>>> but >>>>>>> there is a tracking WebKit issue: >>>>>>> https://bugs.webkit.org/show_bug.cgi?id=201556 >>>>>>> >>>>>>> *Web developers*: Positive ( >>>>>>> https://twitter.com/nghuuphuoc/status/1618806085158051846?s=20) >>>>>>> Other examples: >>>>>>> https://twitter.com/radogado/status/1621479592123826184?s=20 >>>>>>> https://twitter.com/ebidel/status/1621037204297637891?lang=en >>>>>>> >>>>>>> *Other signals*: >>>>>>> >>>>>>> WebView application risks >>>>>>> >>>>>>> Does this intent deprecate or change behavior of existing APIs, such >>>>>>> that it has potentially high risk for Android WebView-based >>>>>>> applications? >>>>>>> >>>>>>> Not potentially high risk. >>>>>>> >>>>>>> >>>>>>> Debuggability >>>>>>> >>>>>>> We verified via Protocol Monitor that DevTools supports breaking on >>>>>>> scrollend listeners, similar to other events. DevTools UI change is >>>>>>> needed >>>>>>> to make this accessible which will be done via crrev.com/c/4376080. >>>>>>> >>>>>>> >>>>>>> Will this feature be supported on all six Blink platforms (Windows, >>>>>>> Mac, Linux, Chrome OS, Android, and Android WebView)? Yes >>>>>>> >>>>>>> Is this feature fully tested by web-platform-tests >>>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md> >>>>>>> ? Yes >>>>>>> >>>>>> >>>>>> >>>>>> https://wpt.fyi/results/dom/events/scrolling?label=master&label=experimental&aligned >>>>>> paints an odd picture, where our current experimental implementation >>>>>> passes >>>>>> some of the tests, but not others, and Firefox seems to be failing some >>>>>> of >>>>>> them. >>>>>> Can you elaborate on the end state you're expecting once this ships? >>>>>> >>>>> We should have filtered that wpt.fyi link to scrollend tests: >>>>> https://wpt.fyi/results/dom/events/scrolling?label=master&label=experimental&aligned&q=scrollend >>>>> We expect the scrollend tests to pass on all the browsers except >>>>> Safari which doesn't support scrollend yet I believe. >>>>> I've looked more closely into the failures and filed crbug.com/1430947 >>>>> explaining the reasons they are failing should have them all fixed soon. >>>>> They are mostly issues with the way the tests are written. >>>>> >>>> >>>> Do you have a sense of the Firefox failures? Are they all in newer >>>> tests that were added after they implemented the feature? >>>> I'm slightly concerned we'd ship this with interoperability issues. >>>> >>> I was able to reproduce the firefox failures locally and I observed 2 >>> main issues: >>> i) In some tests Firefox just isn't scrolling or isn't firing scrollend >>> when we expect it to, even though when I manually interacting with the >>> firefox browser in the same scenarios it does scroll and does fire >>> scrollend as expected. These seem to have to do with the test >>> environment/setup. This affects old tests where it's not yet clear why the >>> outcome is different in tests than via manual interaction. >>> There is an existing mozilla bug for some of these tests: >>> https://bugzilla.mozilla.org/show_bug.cgi?id=1655754. I will update the >>> relevant bugs to see if firefox folks can take a look. There are also newer >>> bugs for more recent tests: >>> https://bugzilla.mozilla.org/show_bug.cgi?id=1814530, >>> https://bugzilla.mozilla.org/show_bug.cgi?id=1826912. I will comment on >>> these bugs as well. >>> >>> ii) Firefox sometimes (i.e. when scrolling via keyboard) fires scrollend >>> when there is no scroll - in this case, there shouldn't be a scrollend >>> event, per the second note in the spec ( >>> https://drafts.csswg.org/cssom-view/#scrolling). With wheel scrolls, >>> firefox (correctly) does not fire scrollend if there is no scroll. I will >>> file a mozilla bug for this issue and update here when I've done so. >>> >>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> Flag name N/A base::Feature is autogenerated from >>>>>>> runtime_enabled_features.json5 >>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/runtime_enabled_features.json5;l=2920?q=%22name:%20%22ScrollEndEvents%22%22&sq=&ss=chromium%2Fchromium%2Fsrc> >>>>>>> >>>>>>> Requires code in //chrome? False >>>>>>> >>>>>>> Tracking bug >>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=907601 >>>>>>> >>>>>>> Estimated milestones >>>>>>> M114 >>>>>>> >>>>>>> Anticipated spec changes >>>>>>> >>>>>>> Open questions about a feature may be a source of future web compat >>>>>>> or interop issues. Please list open issues (e.g. links to known github >>>>>>> issues in the project for the feature specification) whose resolution >>>>>>> may >>>>>>> introduce web compat/interop risk (e.g., changing to naming or >>>>>>> structure of >>>>>>> the API in a non-backward-compatible way). >>>>>>> >>>>>>> >>>>>>> Link to entry on the Chrome Platform Status >>>>>>> https://chromestatus.com/feature/5186382643855360 >>>>>>> >>>>>>> This intent message was generated by Chrome Platform Status >>>>>>> <https://chromestatus.com/>. >>>>>>> >>>>>>> -- >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "blink-dev" group. >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>> send an email to blink-dev+unsubscr...@chromium.org. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAA6pwF7nGNT0bwM8VY3Jj0TAEe9jNptKuwrMN1%3DO8tnqH2t8JQ%40mail.gmail.com >>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAA6pwF7nGNT0bwM8VY3Jj0TAEe9jNptKuwrMN1%3DO8tnqH2t8JQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >> You received this message because you are subscribed to the Google Groups >> "blink-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to blink-dev+unsubscr...@chromium.org. >> >> To view this discussion on the web visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfV6V7eLcqcgSZfiNP%3Dm8gA5YKKGDfJKLiz5Y%2B8iFEDggQ%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfV6V7eLcqcgSZfiNP%3Dm8gA5YKKGDfJKLiz5Y%2B8iFEDggQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to blink-dev+unsubscr...@chromium.org. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/76a74d18-3704-4ca4-8e08-f45be9e95048n%40chromium.org > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/76a74d18-3704-4ca4-8e08-f45be9e95048n%40chromium.org?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw_j-5rC__zRuNjMhht_%2BWLf9KVaXOF8Ee2M_HGYWPrVnA%40mail.gmail.com.
