Bug 36117

Summary: Web Inspector: migrate to native styles inspector in order to inspect styles from foreighn domains.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, dglazkov, eric, joepeck, rik, timothy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed change.
none
[PATCH] Patch to land. none

Description Pavel Feldman 2010-03-15 08:11:15 PDT 
(This will be used for audits too)
Comment 1 Pavel Feldman 2010-03-15 08:13:50 PDT 
Created attachment 50708 [details]
[PATCH] Proposed change.
Comment 2 WebKit Review Bot 2010-03-15 08:16:08 PDT 
Attachment 50708 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/inspector/InspectorDOMAgent.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/InspectorDOMAgent.cpp:773:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/inspector/InspectorDOMAgent.cpp:853:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/inspector/InspectorDOMAgent.cpp:867:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 4 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Eric Seidel (no email) 2010-03-15 08:18:00 PDT 
Attachment 50708 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/869002
Comment 4 WebKit Review Bot 2010-03-15 08:22:06 PDT 
Attachment 50708 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/885001
Comment 5 Timothy Hatcher 2010-03-15 08:45:31 PDT 
Comment on attachment 50708 [details]
[PATCH] Proposed change.

> +    for (unsigned i = 0; …

Use size_t instead for for loops.


> +    bool isUserAgent = parentStyleSheet && !parentStyleSheet->ownerNode() && parentStyleSheet->href().isEmpty();
> +    bool isUser = parentStyleSheet && parentStyleSheet->ownerNode() && parentStyleSheet->ownerNode()->nodeName() == "#document";

I suspect there is a more direct way to get this info now that we are in native code.


I lament to loss of easier to write JavaScript…
Comment 6 Timothy Hatcher 2010-03-15 08:46:22 PDT 
Make sure to fix the flagged style errors and the build errors.
Comment 7 Pavel Feldman 2010-03-15 14:45:07 PDT 
Created attachment 50744 [details]
[PATCH] Patch to land.
Comment 8 Eric Seidel (no email) 2010-03-15 15:55:13 PDT 
Comment on attachment 50708 [details]
[PATCH] Proposed change.

Cleared Timothy Hatcher's review+ from obsolete attachment 50708 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 9 Joseph Pecoraro 2010-03-15 21:13:02 PDT 
(In reply to comment #6)
> Make sure to fix the flagged style errors and the build errors.

Also, is it really "foreighn" or should it be "foreign" or just "any domain"? It would be nice to update the bug + ChangeLog if that is a typo.
Comment 10 Pavel Feldman 2010-03-16 02:33:03 PDT 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/inspector/styles-iframe.html
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/ScriptValue.h
	M	WebCore/bindings/v8/ScriptValue.h
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorBackend.h
	M	WebCore/inspector/InspectorBackend.idl
	M	WebCore/inspector/InspectorDOMAgent.cpp
	M	WebCore/inspector/InspectorDOMAgent.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	M	WebCore/inspector/front-end/DOMAgent.js
	M	WebCore/inspector/front-end/ElementsTreeOutline.js
	M	WebCore/inspector/front-end/InjectedScript.js
	M	WebCore/inspector/front-end/MetricsSidebarPane.js
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/js/InspectorControllerImpl.js
Committed r56049