Bug 78482 - Web Inspector: wrap settings selector text and adding a rule with undoable actions.
Summary: Web Inspector: wrap settings selector text and adding a rule with undoable ac...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on: 78502
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-13 03:51 PST by Pavel Feldman
Modified: 2012-02-13 13:38 PST (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2012-02-13 03:51:58 PST
Current set of actions is incomplete, adding missing ones.
Comment 1 Pavel Feldman 2012-02-13 03:57:23 PST
Created attachment 126748 [details]
Patch
Comment 2 Pavel Feldman 2012-02-13 04:15:31 PST
Created attachment 126750 [details]
[Patch] with no fireStyleSheetChanged part.
Comment 3 Alexander Pavlov (apavlov) 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.)
Comment 4 Pavel Feldman 2012-02-13 05:07:29 PST
Created attachment 126758 [details]
[Patch] review comments addressed.
Comment 5 Pavel Feldman 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.
Comment 6 Pavel Feldman 2012-02-13 05:12:37 PST
Committed r107561: <http://trac.webkit.org/changeset/107561>