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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1960553002: Do not ignore the text style properties when checking styles in EditingStyle. (Closed)

Created:
4 years, 7 months ago by joone
Modified:
4 years, 7 months ago
Reviewers:
yosin_UTC9
CC:
blink-reviews, blink-reviews-style_chromium.org, chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Do not ignore the text style properties when checking styles in EditingStyle. When we run document.queryCommandState() to see if the text styles(bold, italic, underline, strikethrough) were applied on selected img tag, it returns wrong value because the text styles are only considerded for text node. BUG=584939 TEST=editing/execCommand/queryCommandState-03.html Committed: https://crrev.com/1e7d66ca32345881ae135af6ed712d3571f9b9e9 Cr-Commit-Position: refs/heads/master@{#392855}

Patch Set 1 #

Patch Set 2 : move the test case #

Total comments: 1

Patch Set 3 : use w3c test harness #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -1 line) Patch
A third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html View 1 2 1 chunk +19 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EditingStyle.cpp View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 12 (5 generated)
joone
Hi yosin@, could you take a look at this CL?
4 years, 7 months ago (2016-05-06 09:02:45 UTC) #2
yosin_UTC9
C++ changes seems to be good. https://codereview.chromium.org/1960553002/diff/20001/third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html File third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html (right): https://codereview.chromium.org/1960553002/diff/20001/third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html#newcode1 third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html:1: <html> Could you ...
4 years, 7 months ago (2016-05-09 03:43:50 UTC) #3
joone
On 2016/05/09 03:43:50, Yosi_UTC9 wrote: > C++ changes seems to be good. > > https://codereview.chromium.org/1960553002/diff/20001/third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-03.html ...
4 years, 7 months ago (2016-05-10 12:37:11 UTC) #5
yosin_UTC9
lgtm
4 years, 7 months ago (2016-05-11 01:09:41 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1960553002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1960553002/40001
4 years, 7 months ago (2016-05-11 01:10:31 UTC) #8
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 7 months ago (2016-05-11 04:54:21 UTC) #10
commit-bot: I haz the power
4 years, 7 months ago (2016-05-11 04:55:43 UTC) #12
Message was sent while issue was closed. 
Patchset 3 (id:??) landed as
https://crrev.com/1e7d66ca32345881ae135af6ed712d3571f9b9e9
Cr-Commit-Position: refs/heads/master@{#392855}

Powered by Google App Engine
This is Rietveld 408576698