Bug 214230 – font-weight should always serialize as a number

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Takeshi Kurosawa]

Created attachment 404087 [details]
Test case

The font-weight should always serialize as a number (400, 700 and so on). 

WebKit uses keyword ("normal" or "bold") when possible and number otherwise (300, 600 and son on). This behavior has some drawbacks:


1. It is a bit harder to get differences of computed values in animations


```
const {fontWeight: oldFontWeight} = window.getComputedStyle(element);
// do some stuff
const {fontWeight} = window.getComputedStyle(element));
const diff = parseInt(oldFontWeight, 10) - parseInt(fontWeight, 10);
```


Expected behavior
If oldFontWeight is "300" and fontWeight is "600" then the diff is 300.
If oldFontWeight is "400" and fontWeight is "700" then the diff is 300.

Actual behavior
If oldFontWeight is "300" and fontWeight is "600" then the diff  is 300.😀 
If oldFontWeight is "normal" and fontWeight is "bold" then the diff is NaN.🥺


2. Inconsistency with specs

CSS Fonts Module Level 3 defines computed value of font-weight is "numeric"
https://www.w3.org/TR/2018/REC-css-fonts-3-20180920/#propdef-font-weight

CSS Fonts Module Level 4 defines computed value of font-weight is "number"
https://drafts.csswg.org/css-fonts/#propdef-font-weight


3. Inconsistency with other browsers

- IE 11: always use a number
- Chrome: always uses a number
- Firefox always uses a number

WebKit should always use number for font-weight.

[Sam Sneddon [:gsnedders]]

This accounts for all of the failures in https://wpt.fyi/results/css/css-fonts/parsing/font-weight-computed.html?label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned

I presume there's some compat risk here (esp. with regards to WKWebViews and Books), even with other UAs already doing this per spec.

[Radar WebKit Bug Importer]

<rdar://problem/65623540>

[Tyler Wilcock]

Created attachment 408661 [details]
Patch

[Myles C. Maxfield]

The patch looks pretty good, but it looks like there need to be some test updates before it can be committed.

[Tyler Wilcock]

Created attachment 409107 [details]
Patch

This patch should be closer, but I'm still expecting some failures.  I want to see what the results in EWS look like.

[Tyler Wilcock]

This patch has resulted in pixel test changes that I'm not sure are right.

For example, look at the layout test results for ios-wk2 [1].  In the text snapshot of `editing/pasteboard/3976872.html` [2], <span>s are now being generated instead of <b>s, which in itself is not necessarily incorrect, assuming the span is styled correctly.  But in the image diff [3], the result box now ends up being highlighted.  This actually matches Chromium's behavior for this test.

One that certainly seems incorrect, though, is `editing/pasteboard/paste-match-style-001.html`, where the image diff [4] shows a space has been added between the 'a' and the 'b' characters.  The text diff [5] shows both a <b> *and* a <span> are being generated.  Interestingly, neither Chromium nor Gecko render the 'b' character at all for this test.

It seems like these inconsistencies are all related to usage of https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand, which is apparently deprecated.  Should I just update the expected results with these new ones, or do we need to dig deeper on any of these?


[1]: https://ews-build.s3-us-west-2.amazonaws.com/iOS-13-Simulator-WK2-Tests-EWS/r409107-26225/results.html

[2]: https://ews-build.s3-us-west-2.amazonaws.com/iOS-13-Simulator-WK2-Tests-EWS/r409107-26225/editing/pasteboard/3976872-pretty-diff.html

[3]: https://ews-build.s3-us-west-2.amazonaws.com/iOS-13-Simulator-WK2-Tests-EWS/r409107-26225/retries/editing/pasteboard/3976872-diffs.html

[4]: https://ews-build.s3-us-west-2.amazonaws.com/iOS-13-Simulator-WK2-Tests-EWS/r409107-26225/retries/editing/pasteboard/paste-match-style-001-diffs.html

[5]: https://ews-build.s3-us-west-2.amazonaws.com/iOS-13-Simulator-WK2-Tests-EWS/r409107-26225/editing/pasteboard/paste-match-style-001-pretty-diff.html

[Sam Sneddon [:gsnedders]]

*** Bug 228540 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

This hasn't been touched in about a year, and the Assignee field is "Nobody," so if nobody minds, I'll pick this up and continue working on it.

[Tyler Wilcock]

Go for it!

[Myles C. Maxfield]

Created attachment 438193 [details]
Patch

[Myles C. Maxfield]

Created attachment 438216 [details]
Patch

[Myles C. Maxfield]

Created attachment 438218 [details]
Patch

[Myles C. Maxfield]

editing/mac/spelling/autocorrection-blockquote-crash.html fails for me even on a clean build, without this patch applied 🤔

[Myles C. Maxfield]

Oh, it's marked as a failure in LayoutTests/platform/mac-wk2/TestExpectations

[Myles C. Maxfield]

HTMLElementEquivalent::valueIsPresentInStyle() / htmlElementEquivalents() needs to be updated, too.

[Myles C. Maxfield]

Created attachment 438235 [details]
Patch

[Myles C. Maxfield]

Created attachment 438264 [details]
Patch

[EWS]

Committed r282545 (241743@main): <https://commits.webkit.org/241743@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438264 [details].

[Brent Fulgham]

This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta.