Bug 78482

Summary:

Web Inspector: wrap settings selector text and adding a rule with undoable actions.

Product:

WebKit

Reporter:

Pavel Feldman <pfeldman>

Component:

Web Inspector (Deprecated)

Assignee:

Pavel Feldman <pfeldman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, tony, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

78502

Bug Blocks:

Attachments:

Description	Flags
Patch	none
[Patch] with no fireStyleSheetChanged part.	none
[Patch] review comments addressed.	yurys: review+

Pavel Feldman

Reported 2012-02-13 03:51:58 PST

Current set of actions is incomplete, adding missing ones.

Attachments
Patch (45.47 KB, patch) 2012-02-13 03:57 PST, Pavel Feldman	no flags	Details Formatted Diff Diff
[Patch] with no fireStyleSheetChanged part. (30.26 KB, patch) 2012-02-13 04:15 PST, Pavel Feldman	no flags	Details Formatted Diff Diff
[Patch] review comments addressed. (31.36 KB, patch) 2012-02-13 05:07 PST, Pavel Feldman	yurys: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Pavel Feldman

Comment 1 2012-02-13 03:57:23 PST

Created attachment 126748 [details] Patch

Pavel Feldman

Comment 2 2012-02-13 04:15:31 PST

Created attachment 126750 [details] [Patch] with no fireStyleSheetChanged part.

Alexander Pavlov (apavlov)

Comment 3 2012-02-13 04:41:41 PST

Comment on attachment 126750 [details] [Patch] with no fireStyleSheetChanged part. View in context: https://bugs.webkit.org/attachment.cgi?id=126750&action=review > Source/WebCore/inspector/InspectorCSSAgent.cpp:709 > + *errorString = "No stylesheet to add rule to found"; Hard to parse. Perhaps, a simpler "No target stylesheet found"? > Source/WebCore/inspector/InspectorStyleSheet.cpp:798 > + sheetText.remove(sourceData->selectorListRange.start, sourceData->styleSourceData->styleBodyRange.end - sourceData->selectorListRange.start); Please check that this removes the rule body trailing '}' (add the full stylesheet text retrieval to your test.)

Pavel Feldman

Comment 4 2012-02-13 05:07:29 PST

Created attachment 126758 [details] [Patch] review comments addressed.

Pavel Feldman

Comment 5 2012-02-13 05:09:09 PST

(In reply to comment #3) > (From update of attachment 126750 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126750&action=review > > > Source/WebCore/inspector/InspectorCSSAgent.cpp:709 > > + *errorString = "No stylesheet to add rule to found"; > > Hard to parse. Perhaps, a simpler "No target stylesheet found"? > Done. > > Source/WebCore/inspector/InspectorStyleSheet.cpp:798 > > + sheetText.remove(sourceData->selectorListRange.start, sourceData->styleSourceData->styleBodyRange.end - sourceData->selectorListRange.start); > > Please check that this removes the rule body trailing '}' (add the full stylesheet text retrieval to your test.) Done. You actually caught a bug.

Pavel Feldman

Comment 6 2012-02-13 05:12:37 PST

Committed r107561: <http://trac.webkit.org/changeset/107561>

Tony Chang

Comment 7 2012-02-13 12:32:00 PST

inspector/styles/undo-add-new-rule.html is crashing in debug: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=fast%2Fcanvas%2Fcanvas-text-alignment.html

Tony Chang

Comment 8 2012-02-13 13:38:54 PST

(In reply to comment #7) > inspector/styles/undo-add-new-rule.html is crashing in debug: > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=fast%2Fcanvas%2Fcanvas-text-alignment.html Sorry, that was the wrong link: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=inspector%2Fstyles%2Fundo-add-new-rule.html I filed bug 78522 about the test crashing in debug.

Note You need to log in before you can comment on or make changes to this bug.