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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 211193006: DevTools: [CSS] use setStyleText as undo action for setPropertyText (Closed)

Created:
6 years, 9 months ago by lushnikov
Modified:
6 years, 5 months ago
Reviewers:
vsevik, apavlov, yurys
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org
Visibility:
Public.

Description

DevTools: [CSS] use setStyleText as undo action for setPropertyText This patch changes SetPropertyTextAction to use setStyleText method instead of setPropertyText for the undo operation. This makes it possible to successfully undo multiple property insertions. BUG=341506 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170206

Patch Set 1 #

Patch Set 2 : add testcase #

Total comments: 2

Patch Set 3 : address @apavlov comment #

Patch Set 4 : rebaseline #

Patch Set 5 : do not crash renderer in case of invalid styleId #

Total comments: 2

Messages

Total messages: 37 (0 generated)
lushnikov
Please take a look.
6 years, 9 months ago (2014-03-25 19:25:02 UTC) #1
apavlov
The code looks good, but apparently needs a test
6 years, 9 months ago (2014-03-26 11:42:29 UTC) #2
lushnikov
Testcase added :)
6 years, 9 months ago (2014-03-26 13:10:45 UTC) #3
apavlov
lgtm https://codereview.chromium.org/211193006/diff/50001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js File LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js (right): https://codereview.chromium.org/211193006/diff/50001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js#newcode89 LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js:89: fun.call(null, nextTest); Why not fun(nextTest); ?
6 years, 9 months ago (2014-03-26 13:15:38 UTC) #4
lushnikov
thanks for the review https://codereview.chromium.org/211193006/diff/50001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js File LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js (right): https://codereview.chromium.org/211193006/diff/50001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js#newcode89 LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js:89: fun.call(null, nextTest); On 2014/03/26 13:15:38, ...
6 years, 9 months ago (2014-03-26 13:23:22 UTC) #5
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-26 13:23:27 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/110001
6 years, 9 months ago (2014-03-26 13:23:28 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-26 14:28:12 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-26 14:28:13 UTC) #9
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-26 20:09:15 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/110001
6 years, 9 months ago (2014-03-26 20:09:24 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-26 21:14:40 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-26 21:14:41 UTC) #13
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 09:34:25 UTC) #14
lushnikov
The CQ bit was unchecked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 09:34:35 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/110001
6 years, 9 months ago (2014-03-27 09:34:35 UTC) #16
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 09:55:59 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/110001
6 years, 9 months ago (2014-03-27 09:56:08 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-27 11:37:47 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-27 11:37:47 UTC) #20
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 12:14:09 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/170001
6 years, 9 months ago (2014-03-27 12:14:12 UTC) #22
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-27 13:13:19 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 9 months ago (2014-03-27 13:13:19 UTC) #24
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 14:26:27 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/190001
6 years, 9 months ago (2014-03-27 14:26:30 UTC) #26
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-27 14:36:31 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on blink_presubmit
6 years, 9 months ago (2014-03-27 14:36:32 UTC) #28
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 15:14:28 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/190001
6 years, 9 months ago (2014-03-27 15:14:38 UTC) #30
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-27 15:32:35 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_rel
6 years, 9 months ago (2014-03-27 15:32:36 UTC) #32
lushnikov
The CQ bit was checked by lushnikov@chromium.org
6 years, 9 months ago (2014-03-27 19:42:39 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lushnikov@chromium.org/211193006/190001
6 years, 9 months ago (2014-03-27 19:42:50 UTC) #34
commit-bot: I haz the power
Change committed as 170206
6 years, 9 months ago (2014-03-27 20:44:55 UTC) #35
yurys
https://codereview.chromium.org/211193006/diff/190001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js File LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js (right): https://codereview.chromium.org/211193006/diff/190001/LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js#newcode69 LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js:69: InspectorTest.domUndo = function(callback) Why all these methods are in ...
6 years, 5 months ago (2014-07-21 06:19:14 UTC) #36
lushnikov
6 years, 5 months ago (2014-07-24 12:34:39 UTC) #37
Message was sent while issue was closed. 
https://codereview.chromium.org/211193006/diff/190001/LayoutTests/http/tests/...
File LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js
(right):

https://codereview.chromium.org/211193006/diff/190001/LayoutTests/http/tests/...
LayoutTests/http/tests/inspector-protocol/inspector-protocol-test.js:69:
InspectorTest.domUndo = function(callback)
On 2014/07/21 06:19:13, yurys wrote:
> Why all these methods are in this file rather than in css-protocol-test.js ?

It's a general purpose methods.

Powered by Google App Engine
This is Rietveld 408576698