Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(623)

Issue 1820773002: getComputedStyle() should return used value for 'line-height' (Closed)

Created:
4 years, 9 months ago by nainar
Modified:
4 years, 2 months ago
Reviewers:
Timothy Loh
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, chromium-reviews, dcheng, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, philipj_slow, rwlbuis, nessy, vcarbune.chromium
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

getComputedStyle() should return used value for 'line-height' As per the spec https://drafts.csswg.org/cssom/#resolved-value getComputedStyle() should return used value for 'line-height' which Chrome currently doesn't do. This patch fixes that to make it more interoperable with FF. BUG=383409

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 3

Patch Set 7 : #

Patch Set 8 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -116 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/css3/calc/font-size-fractional-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/data-transfer-items-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/dragstart-contains-default-content-expected.txt View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/atapply/at-apply-shorthands.html View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/css2-system-fonts.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/font-shorthand-from-longhands.html View 1 2 3 4 5 2 chunks +8 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/font-shorthand-from-longhands-expected.txt View 1 2 3 4 5 2 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/font-shorthand-line-height.html View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/font-shorthand-line-height-expected.txt View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-font-expected.txt View 1 2 3 4 5 1 chunk +28 lines, -28 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/fast/css/getComputedStyle/getComputedStyle-lineHeight.html View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/resources/computed-style-listing.js View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/script-tests/computed-style-font.js View 1 2 3 4 5 1 chunk +35 lines, -28 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/ondrop-text-html-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/history/visited-link-button.html View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/history/visited-link-button-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/ruby/ruby-line-height-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/ruby/script-tests/ruby-line-height.js View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/media/track/track-cue-rendering-on-resize.html View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/media/track/track-cue-rendering-on-resize-expected.txt View 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/fast/css/css2-system-fonts-expected.txt View 1 2 3 4 5 1 chunk +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/fast/css/css2-system-fonts-expected.txt View 1 2 3 4 5 1 chunk +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/fast/css/css2-system-fonts-expected.txt View 1 2 3 4 5 1 chunk +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 5 6 1 chunk +1 line, -5 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.h View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.cpp View 1 2 3 4 5 6 1 chunk +15 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebViewTest.cpp View 1 2 3 4 5 6 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 20 (4 generated)
nainar
Hi Eddy! PTAL? Thanks!
4 years, 9 months ago (2016-03-21 05:54:34 UTC) #2
Timothy Loh
https://codereview.chromium.org/1820773002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1820773002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode468 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:468: return zoomAdjustedPixelValue(floatValueForLength(length, style.getFontDescription().computedSize()), style); floatValueForLength resolves percentages but if ...
4 years, 9 months ago (2016-03-21 05:56:20 UTC) #3
nainar
Hi! Responded to Tim's comment. PTAL? Thanks! https://codereview.chromium.org/1820773002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1820773002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode468 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:468: return zoomAdjustedPixelValue(floatValueForLength(length, ...
4 years, 9 months ago (2016-03-21 06:21:45 UTC) #5
Timothy Loh
On 2016/03/21 06:21:45, nainar wrote: > Hi! > > Responded to Tim's comment. PTAL? > ...
4 years, 9 months ago (2016-03-21 23:54:12 UTC) #6
nainar
Fixed the failing tests issue. Also changed computedLineHeight to return a float instead of an ...
4 years, 9 months ago (2016-03-22 07:53:31 UTC) #7
nainar
@meade, Changed the needed unit tests. PTAL? Thanks!
4 years, 9 months ago (2016-03-23 03:53:37 UTC) #8
meade_UTC10
On 2016/03/23 03:53:37, nainar wrote: > @meade, > > Changed the needed unit tests. PTAL? ...
4 years, 9 months ago (2016-03-23 23:18:16 UTC) #9
nainar
@timloh, Tim, could you take a look at the code change. I still have a ...
4 years, 8 months ago (2016-04-06 06:39:51 UTC) #10
Timothy Loh
https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h#newcode636 third_party/WebKit/Source/core/style/ComputedStyle.h:636: float computedLineHeight() const; Why is this changing?
4 years, 8 months ago (2016-04-11 03:32:16 UTC) #11
nainar
https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h#newcode636 third_party/WebKit/Source/core/style/ComputedStyle.h:636: float computedLineHeight() const; If I retain it as int ...
4 years, 8 months ago (2016-04-11 04:42:15 UTC) #12
Timothy Loh
https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h#newcode636 third_party/WebKit/Source/core/style/ComputedStyle.h:636: float computedLineHeight() const; On 2016/04/11 04:42:15, nainar wrote: > ...
4 years, 8 months ago (2016-04-11 04:44:43 UTC) #13
nainar
On 2016/04/11 at 04:42:15, nainar wrote: > https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h > File third_party/WebKit/Source/core/style/ComputedStyle.h (right): > > https://codereview.chromium.org/1820773002/diff/90001/third_party/WebKit/Source/core/style/ComputedStyle.h#newcode636 ...
4 years, 8 months ago (2016-04-11 04:53:57 UTC) #14
Timothy Loh
On 2016/04/11 04:53:57, nainar wrote: > On 2016/04/11 at 04:42:15, nainar wrote: > > > ...
4 years, 8 months ago (2016-04-11 04:55:03 UTC) #15
nainar
On 2016/04/11 at 04:55:03, timloh wrote: > On 2016/04/11 04:53:57, nainar wrote: > > On ...
4 years, 8 months ago (2016-04-11 05:41:58 UTC) #16
Timothy Loh
ok I refreshed myself on the relevant specs, in particular getComputedStyle line-height -> resolved value ...
4 years, 8 months ago (2016-04-12 13:25:40 UTC) #17
meade_UTC10
4 years, 4 months ago (2016-08-15 01:04:44 UTC) #18
I'm going to remove myself from the R line if this CL isn't needed any more.

Powered by Google App Engine
This is Rietveld 408576698