LGTM3 But note that our guidelines do require a flag <https://chromium.googlesource.com/chromium/src/+/main/docs/flag_guarding_guidelines.md#When-is-a-flag-required>, and I believe it should be absolutely trivial to do so with a RuntimeEnabledFeature (like maybe 30 characters of typing?). But also in this case I think the risk is very low (tiny obscure API) so I won't withhold my LGTM on verifying the guidance has been followed.
Rick On Wed, Dec 13, 2023 at 11:25 AM Daniel Bratell <bratel...@gmail.com> wrote: > LGTM2 > > I would recommend a flag but given Philip's arguments I'll not consider it > an absolute requirement. > > /Daniel > On 2023-12-13 15:28, Philip Jägenstedt wrote: > > LGTM1 with or without a feature flag. > > I'm comfortable to LGTM this even without a flag because the length > property was exposed in Gecko > <https://github.com/mozilla/gecko-dev/commit/fef2d448e2264502e5dbb2297781db4a2c795b81> > and WebKit > <https://github.com/WebKit/WebKit/commit/e6fb2489c688f12f374f6605820f9927b422ddeb> > quite > recently, so `'length' in CSSKeyframesRule.prototype` is not a reasonable > proxy for "is not Chrome". Firefox 108 and Safari 16 (source > <https://github.com/mdn/browser-compat-data/blob/568fde27290173610f7a0129597a0be83111642d/api/CSSKeyframesRule.json#L306-L339>) > would fall into the same code path, and those are still in wide enough use > that this API doesn't work for engine detection. > > On Mon, Dec 11, 2023 at 3:48 PM Mike Taylor <miketa...@chromium.org> > wrote: > >> This seems like a fairly trivial and useful addition, but can we put this >> behind a feature flag? It's not hard to imagine some code looking at >> `CSSKeyframesRule.length` as a proxy for "is not Chrome" and something >> going wrong (though, I imagine this is a very small risk). >> >> >> https://chromium.googlesource.com/chromium/src/+/main/docs/flag_guarding_guidelines.md >> On 12/8/23 11:50 PM, Amos Lim wrote: >> >> Contact emails eui-sang....@samsung.com >> >> Explainer None >> >> Specification >> https://drafts.csswg.org/css-animations/#interface-csskeyframesrule >> >> Summary >> >> Exposes length attribute of CSSKeyframesRule. Interfaces that support >> indexed properties must define an integer-typed attribute named "length". >> >> >> Blink component Blink>CSS >> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3ECSS> >> >> TAG review None >> >> TAG review status Not applicable >> >> Risks >> >> >> Interoperability and Compatibility >> >> None >> >> >> *Gecko*: Shipped/Shipping >> >> *WebKit*: Shipped/Shipping >> >> *Web developers*: No signals >> >> *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? >> >> None >> >> >> Debuggability >> >> None >> >> >> Will this feature be supported on all six Blink platforms (Windows, Mac, >> Linux, ChromeOS, 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/css/css-animations/idlharness.html >> https://wpt.fyi/results/css/cssom/CSSKeyframesRule.html >> >> >> Flag name on chrome://flags None >> >> Finch feature name None >> >> Non-finch justification None >> >> Requires code in //chrome? False >> >> Tracking bug >> https://bugs.chromium.org/p/chromium/issues/detail?id=1502758 >> >> Estimated milestones >> >> No milestones specified >> >> >> 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). >> None >> >> Link to entry on the Chrome Platform Status >> https://chromestatus.com/feature/6289894144212992 >> >> 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/CAOGEg00LjpMvMpkdeOQWaLoCsgeCrDMck9wMP4wvX7ttMEE3SQ%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOGEg00LjpMvMpkdeOQWaLoCsgeCrDMck9wMP4wvX7ttMEE3SQ%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/cc41c466-0562-455c-a6de-2ac2094693de%40chromium.org >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/cc41c466-0562-455c-a6de-2ac2094693de%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/CAARdPYdxEsMXCWFkipT_MO%2B9UvEduQu4x-LSD2FN2s%3Dg9kTfSw%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYdxEsMXCWFkipT_MO%2B9UvEduQu4x-LSD2FN2s%3Dg9kTfSw%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/ae0da666-f830-43fc-b9de-2ed2d7225787%40gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ae0da666-f830-43fc-b9de-2ed2d7225787%40gmail.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/CAFUtAY8GMOSgmYfhVZ1C%3Dt4G1_Xo02BnWf5t0HQWHnH%3Dj-VhQA%40mail.gmail.com.
