56310 – Web Inspector: Fix handling of the CSSAgent.setStyleSheetText() results in CSSStyleModel.js

Bug 56310 - Web Inspector: Fix handling of the CSSAgent.setStyleSheetText() results in CSSStyleModel.js

Summary: Web Inspector: Fix handling of the CSSAgent.setStyleSheetText() results in CS...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Web Inspector (Deprecated) (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Alexander Pavlov (apavlov)

URL:
Keywords:

Depends on:	56662 57096
Blocks:
	Show dependency tree / graph

Reported:	2011-03-14 08:16 PDT by Alexander Pavlov (apavlov)
Modified:	2011-03-29 08:39 PDT (History)
CC List:	13 users (show)

See Also:

Attachments
[PATCH] Suggested fix (2.84 KB, patch) 2011-03-14 08:28 PDT, Alexander Pavlov (apavlov)	pfeldman: review- pfeldman: commit-queue-	Details \| Formatted Diff \| Diff
[PATCH] Test added (9.76 KB, patch) 2011-03-14 09:25 PDT, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
[PATCH] Comments addressed (14.94 KB, patch) 2011-03-15 09:51 PDT, Alexander Pavlov (apavlov)	no flags	Details \| Formatted Diff \| Diff
[PATCH] Style fixed (23.42 KB, patch) 2011-03-15 11:35 PDT, Alexander Pavlov (apavlov)	pfeldman: review+	Details \| Formatted Diff \| Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Alexander Pavlov (apavlov) 2011-03-14 08:16:57 PDT

The current implementation:
1. Does not follow the setStyleSheetText() call result specified by the protocol (should be a boolean instead of stylesheet payload)
2. Does not dispatch the "stylesheet changed" event

Comment 1 Alexander Pavlov (apavlov) 2011-03-14 08:28:49 PDT

Created attachment 85678 [details]
[PATCH] Suggested fix

Comment 2 Pavel Feldman 2011-03-14 08:34:35 PDT

Comment on attachment 85678 [details]
[PATCH] Suggested fix

Please add a test.

Comment 3 Alexander Pavlov (apavlov) 2011-03-14 09:25:47 PDT

Created attachment 85684 [details]
[PATCH] Test added

Comment 4 Pavel Feldman 2011-03-14 10:04:40 PDT

Comment on attachment 85684 [details]
[PATCH] Test added

View in context: https://bugs.webkit.org/attachment.cgi?id=85684&action=review

> LayoutTests/inspector/styles/styles-stylesheet-text.html:1
> +<html>

can be get-set-stylesheet-text.html

> LayoutTests/inspector/styles/styles-stylesheet-text.html:24
> +    function test_findStyleSheet(error, styleSheetIds)

use InspectorTest.runTestSuite for the reset instead.

Also, I think getAllStyles should be called getAllStyleSheets and return stylesheet metainfo ('url' for now) along with its id.

Comment 5 Alexander Pavlov (apavlov) 2011-03-15 09:51:19 PDT

Created attachment 85819 [details]
[PATCH] Comments addressed

Comment 6 WebKit Review Bot 2011-03-15 09:53:28 PDT

Attachment 85819 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebCore/inspector/InspectorCSSAgent.h:67:  The parameter name "error" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 Alexander Pavlov (apavlov) 2011-03-15 11:35:40 PDT

Created attachment 85832 [details]
[PATCH] Style fixed

Comment 8 WebKit Review Bot 2011-03-18 11:38:43 PDT

http://trac.webkit.org/changeset/81487 might have broken Leopard Intel Release (Tests)
The following tests are not passing:
inspector/styles/styles-add-blank-property.html

Comment 9 WebKit Review Bot 2011-03-25 05:21:01 PDT

http://trac.webkit.org/changeset/81953 might have broken GTK Linux 32-bit Release
The following tests are not passing:
inspector/styles/styles-add-blank-property.html

Comment 10 Alexander Pavlov (apavlov) 2011-03-29 08:39:03 PDT

Landed with a flakiness fix for LayoutTests/inspector/styles/styles-add-blank-property.html from bug 57252.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       LayoutTests/ChangeLog
        A       LayoutTests/inspector/styles/get-set-stylesheet-text-expected.txt
        A       LayoutTests/inspector/styles/get-set-stylesheet-text.html
        A       LayoutTests/inspector/styles/resources/get-set-stylesheet-text.css
        M       LayoutTests/inspector/styles/styles-add-blank-property.html
        M       LayoutTests/inspector/styles/styles-new-API.html
        M       Source/WebCore/ChangeLog
        M       Source/WebCore/inspector/Inspector.json
        M       Source/WebCore/inspector/InspectorCSSAgent.cpp
        M       Source/WebCore/inspector/InspectorCSSAgent.h
        M       Source/WebCore/inspector/InspectorStyleSheet.cpp
        M       Source/WebCore/inspector/InspectorStyleSheet.h
        M       Source/WebCore/inspector/front-end/AuditRules.js
        M       Source/WebCore/inspector/front-end/CSSStyleModel.js
Committed r82252