Bug 201296 - getComputedStyle for line-height: normal should return the keyword instead of a length
Summary: getComputedStyle for line-height: normal should return the keyword instead of...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-29 02:51 PDT by Simon Pieters
Modified: 2019-09-09 18:58 PDT (History)
13 users (show)

See Also:


Attachments
Patch (10.62 KB, patch)
2019-09-06 10:03 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Update commit message (10.98 KB, patch)
2019-09-06 10:57 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews212 for win-future (14.14 MB, application/zip)
2019-09-06 12:20 PDT, Build Bot
no flags Details
Update Layout Test Results (83.65 KB, patch)
2019-09-08 03:15 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff
Update ios Layout test result of line-height-boosting.html (85.26 KB, patch)
2019-09-08 04:57 PDT, Joonghun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters 2019-08-29 02:51:34 PDT
From https://github.com/w3c/csswg-drafts/issues/3749#issuecomment-477241555 

> The CSS Working Group just discussed A question for the procedure to compute the resolved value of "line-height", and agreed to the following:
>
> RESOLVED: return 'normal' for line-height

This change was implemented in Gecko in https://bugzilla.mozilla.org/show_bug.cgi?id=1536871

Blink already does this.

(The CSSOM spec hasn't yet been edited to reflect this change.)

Tests: http://web-platform-tests.live/html/rendering/non-replaced-elements/form-controls/resets.html (these were intended to test something else, but fail in WebKit due to this bug.)

Current test results https://wpt.fyi/results/html/rendering/non-replaced-elements/form-controls/resets.html?sha=3ac7cc537e
Comment 1 Joonghun Park 2019-09-05 22:29:44 PDT
The issue https://github.com/w3c/csswg-drafts/issues/3749
had reached its conclusion as Simon commented.

> RESOLVED: return 'normal' for line-height

Per https://github.com/w3c/csswg-drafts/issues/4249 Emillio's comment,
"Firefox has this behavior behind a pref on Nightly / Beta only. I plan to turn it on-by-default on the next release cycle if no regressions are found. This was done in https://bugzilla.mozilla.org/show_bug.cgi?id=1536871 as a result of the discussion in #3749",

I think we can proceed with this issue to make WebKit to return "normal" in gCS for line-height.
Comment 3 Joonghun Park 2019-09-06 00:08:05 PDT
(In reply to Eric Willigers from comment #2)
> WPT: https://wpt.fyi/results/css/css-inline/parsing/line-height-computed.html

Thank you for the reference, Eric:)

More related wpt:
css/cssom/getComputedStyle-line-height.html
html/rendering/replaced-elements/the-select-element/select-1-line-height.html
Comment 4 Joonghun Park 2019-09-06 10:03:39 PDT
Created attachment 378203 [details]
Patch
Comment 5 Joonghun Park 2019-09-06 10:57:00 PDT
Created attachment 378206 [details]
Update commit message
Comment 6 Build Bot 2019-09-06 12:20:30 PDT
Comment on attachment 378206 [details]
Update commit message

Attachment 378206 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13006498

New failing tests:
fast/css/font-shorthand-from-longhands.html
fast/css/getComputedStyle/computed-style-font.html
fast/ruby/ruby-line-height.html
css3/calc/line-height.html
fast/css/font-calculated-value.html
Comment 7 Build Bot 2019-09-06 12:20:34 PDT
Created attachment 378215 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 8 Joonghun Park 2019-09-08 03:15:22 PDT
Created attachment 378321 [details]
Update Layout Test Results
Comment 9 Joonghun Park 2019-09-08 04:57:53 PDT
Created attachment 378322 [details]
Update ios Layout test result of line-height-boosting.html
Comment 10 Joonghun Park 2019-09-09 16:19:49 PDT
Could you please review this patch? I think mac-debug-wk1's webgl/1.0.2/conformance/context/context-release-upon-reload.html failure is not related with this patch.
Comment 11 Joonghun Park 2019-09-09 17:33:20 PDT
Thank you for your review, rniwa:)
Comment 12 WebKit Commit Bot 2019-09-09 18:57:30 PDT
Comment on attachment 378322 [details]
Update ios Layout test result of line-height-boosting.html

Clearing flags on attachment: 378322

Committed r249686: <https://trac.webkit.org/changeset/249686>
Comment 13 WebKit Commit Bot 2019-09-09 18:57:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2019-09-09 18:58:20 PDT
<rdar://problem/55207849>
Comment 15 Radar WebKit Bug Importer 2019-09-09 18:58:22 PDT
<rdar://problem/55207850>