LGTM1 On Mon, Jan 16, 2023 at 10:19 AM 'Munira Tursunova' via blink-dev < blink-dev@chromium.org> wrote:
> Contact emails > > > *moon...@google.com <moon...@google.com>, dr...@google.com > <dr...@google.com>*Explainer > > > *https://github.com/w3c/csswg-drafts/issues/7832#issuecomment-1338671264 > <https://github.com/w3c/csswg-drafts/issues/7832#issuecomment-1338671264>https://github.com/w3c/csswg-drafts/issues/1636 > <https://github.com/w3c/csswg-drafts/issues/1636>https://bugs.chromium.org/p/chromium/issues/detail?id=1379236 > <https://bugs.chromium.org/p/chromium/issues/detail?id=1379236>* > Specification > > > *https://www.w3.org/TR/css-fonts-4/#font-prop > <https://www.w3.org/TR/css-fonts-4/#font-prop>*Summary > > *When the font shorthand is specified all of its subproperties including > font-size-adjust, font-kerning, font-feature-settings, font-optical-sizing > and font-variation-settings should be reset to their initial values.Since > Chrome wasn’t resetting some font subproperties before, there is a risk > that content would rely on this bug. Therefore the ClusterTelemetry > analysis was done for the top 10K webpages. Analysis of the > ClusterTelemetry results can be found here > <https://docs.google.com/document/d/1ZYjaZBthiN-yNioJkoqh-aqwC1NOgC-YpptiNlzfdZQ/edit?usp=sharing&resourcekey=0-h32BsErkkx1YizNffkR7LQ>. > The results show it only affects 3 out of ~10K webpages and the changes in > these 3 webpages can be considered minor. Furthermore, Firefox has already > shipped this feature and it was also shipped in Safari Technology preview, > so it will be soon shipped in Safari as well. This suggests the change can > be safely landed on Chrome.* > Thanks for the detailed investigation. 3/10K is 0.03% which is an order of magnitude more than we are typically comfortable with breaking changes. But at the same time, looking at the examples, it's very hard to notice the "brokenness", so I think it's reasonable to assume that many users won't notice it. It may be hard for developers to figure out the source of the difference. Would you consider adding a devtools issue that can make that easier? Motivation > > > > > *When font shorthand is specified, then it means that the font has > changed, so there is no point to apply the same set of settings for the new > font (for example, the new font might not have features that were specified > before or they may look completely different in the new font). Thus if the > properties font-size-adjust, font-kerning, font-feature-settings, > font-optical-sizing and font-variation-settings were set to non-initial > value before, they should be reset.Chrome was already resetting some of > it’s subproperties, like font-variant-*. In CSS WG issue #7832 > <https://github.com/w3c/csswg-drafts/issues/7832#issuecomment-1338671264> > it was recently resolved which of the font subproperties should be reset to > their initial values. So this change adds resetting for font-size-adjust, > font-kerning, font-feature-settings, font-optical-sizing and > font-variation-settings. Firefox and Safari have already implemented this > feature.*Blink component > > > *Blink>Fonts > <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EFonts>*Search > tags > > > *font shorthand <https://chromestatus.com/features#tags:font%20shorthand>, > reset <https://chromestatus.com/features#tags:reset>, font-size-adjust > <https://chromestatus.com/features#tags:font-size-adjust>, font-kerning > <https://chromestatus.com/features#tags:font-kerning>, > font-feature-settings > <https://chromestatus.com/features#tags:font-feature-settings>, > font-optical-sizing > <https://chromestatus.com/features#tags:font-optical-sizing>, > font-variation-settings > <https://chromestatus.com/features#tags:font-variation-settings>*TAG > review > > > *Already shipped in other browsers, see below, no TAG review required.*TAG > review status > > > *Not applicable, existing standard, shipped in other UAs*Risks > Interoperability and Compatibility > > > > *Low, feature is already shipping in Firefox (teste in Firefox 108.0.1) > and Safari (the feature is shipped in Firefox and Safari Technology > Preview). According to results of ClusterTelemetry run on top 10K webpages, > this feature affects only 3 from ~10K webpages and doesn’t cause major > visible page breakages. More information can be found in ClusterTelemetry > results analysis > <https://docs.google.com/document/d/1ZYjaZBthiN-yNioJkoqh-aqwC1NOgC-YpptiNlzfdZQ/edit?usp=sharing&resourcekey=0-h32BsErkkx1YizNffkR7LQ>.Gecko: > Shipped/Shipping > (https://github.com/w3c/csswg-drafts/issues/1636#issuecomment-319966794 > <https://github.com/w3c/csswg-drafts/issues/1636#issuecomment-319966794>) > Gecko > is already resetting all the required font subproperties, shown in the link > above,font-synthesis-* should be "Cascaded Independently" according to spec > <https://www.w3.org/TR/css-fonts-4/#font-prop>: > https://github.com/w3c/csswg-drafts/issues/7832#issuecomment-1338671264 > <https://github.com/w3c/csswg-drafts/issues/7832#issuecomment-1338671264>Also > tested in Firefox stable 108.0.1, the feature is working.WebKit: > Shipped/Shipping > (https://github.com/WebKit/WebKit/commit/9de002d111c74dfffa39582a67d6af89b9abf21c#diff-eeea910564f86ae74d641501cfc2459d985b2446fd9115817b0b1f7763d60589 > <https://github.com/WebKit/WebKit/commit/9de002d111c74dfffa39582a67d6af89b9abf21c#diff-eeea910564f86ae74d641501cfc2459d985b2446fd9115817b0b1f7763d60589> > https://developer.apple.com/safari/technology-preview/release-notes/ > <https://developer.apple.com/safari/technology-preview/release-notes/>)The > feature was shipped in Safari Technology Preview 160, not yet in Safari.* > Activation > > > > *None expected; Feature already implemented in other browsers.* > Debuggability > > > *Same as any other CSS property, font shorthand and its subproperties can > be inspected in DevTools.*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, added new wpt tests in addition to existing.*Flag name > > > *-*Requires code in //chrome? > > > *False*Tracking bug > > > *https://crbug.com/1379236 <https://crbug.com/1379236>*Estimated > milestones > > > > *No milestones specified*Anticipated spec changes > > > > *None expected*Link to entry on the Chrome Platform Status > > https://chromestatus.com/feature/5990758601981952 > > -- > 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/CAAO7W_AsqQve9GHNwpjUQWgdMHkchhh0TS1xM395%2Biq%3D-vO71g%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAO7W_AsqQve9GHNwpjUQWgdMHkchhh0TS1xM395%2Biq%3D-vO71g%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/CAL5BFfXW%2Ba5LycWwPCgZwFWiO55GkYQ1rO8JTL5r2iP9oBmz3w%40mail.gmail.com.
