Chromium Code Reviews
Help | Chromium Project | Sign in
(9)

Issue 2577633002: [css-ui] Update resolved value of caret-color property (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 1 week ago by Manuel Rego
Modified:
2 months, 1 week ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, blink-reviews-w3ctests_chromium.org, chromium-reviews, dglazkov+blink, florian_rivoal.net, rwlbuis, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-ui] Update resolved value of caret-color property The resolved value of caret-color should be the used value, like for the rest of color properties. This was discussed and resolved by the CSS WG (see https://github.com/w3c/csswg-drafts/issues/781). BUG=665422 TEST=imported/csswg-test/css-ui-3/caret-color-009.html TEST=imported/csswg-test/css-ui-3/caret-color-013.html Review-Url: https://codereview.chromium.org/2577633002 Cr-Commit-Position: refs/heads/master@{#442214} Committed: https://chromium.googlesource.com/chromium/src/+/1d659157007dc8af77915b4af72f577b324c5e81

Patch Set 1 #

Patch Set 2 : Fix some tests #

Patch Set 3 : Fix editing tests #

Patch Set 4 : Fix tests #

Total comments: 1

Patch Set 5 : Rebased test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -41 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/dragstart-contains-default-content-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt View 1 2 3 4 1 chunk +1 line, -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 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/ondrop-text-html-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 1 chunk +7 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EditingStyle.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/editing/EditingStyle.cpp View 1 2 3 4 2 chunks +6 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebViewTest.cpp View 1 2 3 4 2 chunks +22 lines, -25 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 17 (6 generated)
Manuel Rego
3 months, 1 week ago (2016-12-14 10:17:16 UTC) #2
chrishtr
Going to defer on this code review also. I'm not the best expert on this ...
3 months ago (2016-12-19 22:30:24 UTC) #4
Yuta Kitamura
https://codereview.chromium.org/2577633002/diff/60001/third_party/WebKit/Source/core/editing/EditingStyle.cpp File third_party/WebKit/Source/core/editing/EditingStyle.cpp (right): https://codereview.chromium.org/2577633002/diff/60001/third_party/WebKit/Source/core/editing/EditingStyle.cpp#newcode510 third_party/WebKit/Source/core/editing/EditingStyle.cpp:510: m_mutableStyle->removeProperty(CSSPropertyCaretColor); I don't think this is correct. * currentColor ...
3 months ago (2016-12-20 09:23:03 UTC) #5
Yuta Kitamura
I'm not totally sure why the CSS semantics change would affect editing.
3 months ago (2016-12-20 09:35:41 UTC) #6
Manuel Rego
Trying to explain this as I agree it's somehow tricky. The problem of changing the ...
3 months ago (2016-12-22 12:54:06 UTC) #7
Manuel Rego
On 2016/12/22 12:54:06, Manuel Rego wrote: > Trying to explain this as I agree it's ...
3 months ago (2016-12-22 13:06:01 UTC) #8
Manuel Rego
Now that the modifications in the tests have been merged upstream: https://github.com/w3c/csswg-test/pull/1161 I've uploaded a ...
2 months, 3 weeks ago (2016-12-26 09:56:37 UTC) #10
Timothy Loh
lgtm
2 months, 1 week ago (2017-01-09 05:33:10 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2577633002/80001
2 months, 1 week ago (2017-01-09 07:47:30 UTC) #13
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/1d659157007dc8af77915b4af72f577b324c5e81
2 months, 1 week ago (2017-01-09 09:14:30 UTC) #16
Manuel Rego
2 months, 1 week ago (2017-01-09 09:34:32 UTC) #17
Message was sent while issue was closed.
JFTR, this CL should point to bug #676763 instead of bug #665422.
Sorry about the mistake.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d1a128a62