Bug 38255 - Web Inspector: REGRESSION: Disabled style properties are absent in Styles sidebar after WebInspector is re-opened
Summary: Web Inspector: REGRESSION: Disabled style properties are absent in Styles sid...
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: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-28 04:49 PDT by Alexander Pavlov (apavlov)
Modified: 2010-05-04 03:29 PDT (History)
10 users (show)

See Also:


Attachments
[PATCH] Suggested solution (5.78 KB, patch)
2010-04-28 08:37 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Factored out the style-related storage (27.28 KB, patch)
2010-04-30 05:07 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Style fixed, change description augmented (27.48 KB, patch)
2010-04-30 05:54 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Added lost WebCore.pro change (27.83 KB, patch)
2010-04-30 06:24 PDT, Alexander Pavlov (apavlov)
pfeldman: review-
pfeldman: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Reverted the lifespan of InspectorDOMAgent (26.27 KB, patch)
2010-04-30 08:34 PDT, Alexander Pavlov (apavlov)
yurys: review+
apavlov: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2010-04-28 04:49:51 PDT
1. Disable a style property in Web Inspector
2. Close and re-open Web Inspector

The disabled style property is missing in the Styles sidebar (rather than rendered disabled).
Comment 1 Alexander Pavlov (apavlov) 2010-04-28 08:37:43 PDT
Created attachment 54567 [details]
[PATCH] Suggested solution
Comment 2 Pavel Feldman 2010-04-28 09:08:22 PDT
Comment on attachment 54567 [details]
[PATCH] Suggested solution

Clearing cq+ since I want to doublecheck something.
Comment 3 Pavel Feldman 2010-04-28 09:16:31 PDT
Comment on attachment 54567 [details]
[PATCH] Suggested solution

A couple of style nits so that InspectorDOMAgent was inline with InspectorController.
Comment 4 Pavel Feldman 2010-04-28 09:21:00 PDT
Comment on attachment 54567 [details]
[PATCH] Suggested solution

Oops, lost the comments with the new inline feature.

WebCore/inspector/InspectorController.cpp:424
 +  void InspectorController::updateDOMAgent()
I'd suggest this to be a domAgent lazy getter.

WebCore/inspector/InspectorController.cpp:439
 +      updateDOMAgent();
Now that dom agent lives longer, this should be domAgent()->setFrontend(m_frontend) instead of constructor with parameter.

and you should simply delegate disconnectFrontend event to the dom agent from the corresponding InspectorController method.
Comment 5 Alexander Pavlov (apavlov) 2010-04-30 05:07:37 PDT
Created attachment 54797 [details]
[PATCH] Factored out the style-related storage
Comment 6 WebKit Review Bot 2010-04-30 05:10:26 PDT
Attachment 54797 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/inspector/InspectorDOMAgent.cpp:79:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/inspector/InspectorController.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/InspectorCSSStore.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Early Warning System Bot 2010-04-30 05:26:48 PDT
Attachment 54797 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1841227
Comment 8 Alexander Pavlov (apavlov) 2010-04-30 05:54:12 PDT
Created attachment 54799 [details]
[PATCH] Style fixed, change description augmented
Comment 9 Early Warning System Bot 2010-04-30 06:09:42 PDT
Attachment 54799 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1938019
Comment 10 Alexander Pavlov (apavlov) 2010-04-30 06:24:08 PDT
Created attachment 54800 [details]
[PATCH] Added lost WebCore.pro change
Comment 11 Yury Semikhatsky 2010-04-30 07:20:40 PDT
Comment on attachment 54800 [details]
[PATCH] Added lost WebCore.pro change

WebCore/inspector/InspectorCSSStore.cpp:45
 +      reset();
Is it necessary?

WebCore/inspector/InspectorCSSStore.h:44
 +  typedef std::pair<String, String> PropertyValueAndPriority;
Consider moving these declarations into the struct below and impporting them with "using"
Comment 12 Alexander Pavlov (apavlov) 2010-04-30 07:27:41 PDT
(In reply to comment #11)
> (From update of attachment 54800 [details])
> WebCore/inspector/InspectorCSSStore.cpp:45
>  +      reset();
> Is it necessary?

Removed

> WebCore/inspector/InspectorCSSStore.h:44
>  +  typedef std::pair<String, String> PropertyValueAndPriority;
> Consider moving these declarations into the struct below and impporting them
> with "using"

As discovered, this is not feasible
Comment 13 Pavel Feldman 2010-04-30 07:53:08 PDT
Comment on attachment 54800 [details]
[PATCH] Added lost WebCore.pro change

I thought we agreed not to change the InspectorDOMAgent lifetime.
Comment 14 Alexander Pavlov (apavlov) 2010-04-30 08:34:35 PDT
Created attachment 54806 [details]
[PATCH] Reverted the lifespan of InspectorDOMAgent
Comment 15 Eric Seidel (no email) 2010-05-02 19:21:51 PDT
Comment on attachment 54567 [details]
[PATCH] Suggested solution

Cleared Yury Semikhatsky's review+ from obsolete attachment 54567 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 16 Alexander Pavlov (apavlov) 2010-05-04 03:29:05 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/GNUmakefile.am
        M       WebCore/WebCore.gypi
        M       WebCore/WebCore.pro
        M       WebCore/WebCore.vcproj/WebCore.vcproj
        M       WebCore/WebCore.xcodeproj/project.pbxproj
        A       WebCore/inspector/InspectorCSSStore.cpp
        A       WebCore/inspector/InspectorCSSStore.h
        M       WebCore/inspector/InspectorController.cpp
        M       WebCore/inspector/InspectorController.h
        M       WebCore/inspector/InspectorDOMAgent.cpp
        M       WebCore/inspector/InspectorDOMAgent.h
Committed r58748