Bug 56310

Summary:

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

Product:

WebKit

Reporter:

Alexander Pavlov (apavlov) <apavlov>

Component:

Web Inspector (Deprecated)

Assignee:

Alexander Pavlov (apavlov) <apavlov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, apavlov, bweinstein, eric, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

56662, 57096

Bug Blocks:

Attachments:

Description	Flags
[PATCH] Suggested fix	pfeldman: review-, pfeldman: commit-queue-
[PATCH] Test added	none
[PATCH] Comments addressed	none
[PATCH] Style fixed	pfeldman: review+

Alexander Pavlov (apavlov)

Reported 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

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 attachment proposed patch, testcase, etc.

Alexander Pavlov (apavlov)

Comment 1 2011-03-14 08:28:49 PDT

Created attachment 85678 [details] [PATCH] Suggested fix

Pavel Feldman

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

Comment on attachment 85678 [details] [PATCH] Suggested fix Please add a test.

Alexander Pavlov (apavlov)

Comment 3 2011-03-14 09:25:47 PDT

Created attachment 85684 [details] [PATCH] Test added

Pavel Feldman

Comment 4 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.

Alexander Pavlov (apavlov)

Comment 5 2011-03-15 09:51:19 PDT

Created attachment 85819 [details] [PATCH] Comments addressed

WebKit Review Bot

Comment 6 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.

Alexander Pavlov (apavlov)

Comment 7 2011-03-15 11:35:40 PDT

Created attachment 85832 [details] [PATCH] Style fixed

WebKit Review Bot

Comment 8 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

WebKit Review Bot

Comment 9 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

Alexander Pavlov (apavlov)

Comment 10 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

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