LGTM3

On 1/16/23 3:44 PM, Rick Byers wrote:
Thanks for the careful compat analysis! LGTM2 - feels like a bugfix-level change to me, because:

  * Severity of breakage
    
<https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.u5ya6jvru7dl>
 seems
    likely to always be very low, minor tweaks to rendered text
  * Ease of adaptation
    
<https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.x5bhg5grhfeo>
    seems high - just do the obvious thing and update the 'font' style
  * Important for interoperability
    
<https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.4hjbxw7513sw>
  * I can imagine no reason why enterprises
    
<https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.axcg738lzcs9>
    or webview would be any more likely to rely on this little
    rendering quirk than random websites

But in case we're wrong about any of these things, please keep an eye on bug reports during the dev and beta channel phases <https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.br2x161kh4n> and circle back here if you get even a single report of non-trivial breakage (as such reports in beta tend to indicate there's an order of magnitude more like it waiting to be hit at stable).

Rick

On Mon, Jan 16, 2023 at 11:51 AM Yoav Weiss <yoavwe...@chromium.org> wrote:

    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, 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
                <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
    
<https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfXW%2Ba5LycWwPCgZwFWiO55GkYQ1rO8JTL5r2iP9oBmz3w%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/CAFUtAY8MXcs%2BQUXX95rcoMjQMwsxmPyrFR3cQeBaTZB-gcARbg%40mail.gmail.com <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY8MXcs%2BQUXX95rcoMjQMwsxmPyrFR3cQeBaTZB-gcARbg%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/f6c0a052-d84f-dc4d-8fdb-b3f1cc6ef5ca%40chromium.org.

Reply via email to