Bug 51478

Summary: Web Inspector: Inline HTML style property out of sync with element.style in Sidebar
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Suggested fix
yurys: review+
[PATCH] Comments addressed yurys: review+

Description Joseph Pecoraro 2010-12-22 09:59:58 PST
Steps to Reproduce:

  1. Inspect <body> on webkit.org
  2. Add style in element.style "height: 20px" (commit with enter)
  3. Edit inline HTML style property to be "height: 10px" (commit with enter)
    => element.style in Sidebar was NOT updated

Results:

  Changes made via the sidebar update immediately in the inline style attribute.
  However, changes made via the inline style attribute, are not immediately
  reflected in the style sidebar.
Comment 1 Alexander Pavlov (apavlov) 2010-12-24 02:37:31 PST
Created attachment 77409 [details]
[PATCH] Suggested fix
Comment 2 Yury Semikhatsky 2010-12-24 05:25:02 PST
Comment on attachment 77409 [details]
[PATCH] Suggested fix

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

> LayoutTests/inspector/elements-delete-inline-style.html:3
> +<script src="../http/tests/inspector/inspector-test.js"></script>

Is it possible to rewrite this test using inspector-test2.js ?

> LayoutTests/inspector/elements-delete-inline-style.html:29
> +function frontend_dumpInspectedStyle(testController)

Method name is a bit misleading since the method does more than just dump style.

> LayoutTests/inspector/elements-delete-inline-style.html:60
> +Tests that the "style" attribute removal results in the Styles sidebar pane update (not a crash).

Description should contain link to the corresponding bug.

> WebCore/ChangeLog:12
> +        (StyledElement::m_inlineStyleDecl), while the code used to rely on it being immutable (hence a crash).

Please add a link to the bug, describing the crash.

> WebCore/inspector/InspectorStyleSheet.h:240
> +    void elementAttributeModified();

For consistency with the rest code I'd rename this method to didModifyElementAtribute
Comment 3 Alexander Pavlov (apavlov) 2010-12-24 07:47:25 PST
This issue is somewhat related to bug 50868, in the sense that removing the "style" element property altogether results in a crash with the same stacktrace.
Comment 4 Alexander Pavlov (apavlov) 2010-12-24 08:22:50 PST
(In reply to comment #2)
> (From update of attachment 77409 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77409&action=review
> 
> > LayoutTests/inspector/elements-delete-inline-style.html:3
> > +<script src="../http/tests/inspector/inspector-test.js"></script>
> 
> Is it possible to rewrite this test using inspector-test2.js ?

Done

> > LayoutTests/inspector/elements-delete-inline-style.html:29
> > +function frontend_dumpInspectedStyle(testController)
> 
> Method name is a bit misleading since the method does more than just dump style.

Done

> > LayoutTests/inspector/elements-delete-inline-style.html:60
> > +Tests that the "style" attribute removal results in the Styles sidebar pane update (not a crash).
> 
> Description should contain link to the corresponding bug.

Done

> > WebCore/ChangeLog:12
> > +        (StyledElement::m_inlineStyleDecl), while the code used to rely on it being immutable (hence a crash).
> 
> Please add a link to the bug, describing the crash.

Added a comment to this bug.

> > WebCore/inspector/InspectorStyleSheet.h:240
> > +    void elementAttributeModified();
> 
> For consistency with the rest code I'd rename this method to didModifyElementAtribute

Done
Comment 5 Alexander Pavlov (apavlov) 2010-12-24 08:23:39 PST
Created attachment 77415 [details]
[PATCH] Comments addressed
Comment 6 Yury Semikhatsky 2010-12-24 08:29:39 PST
Comment on attachment 77415 [details]
[PATCH] Comments addressed

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

Thanks!

> LayoutTests/inspector/elements-delete-inline-style.html:54
>  \ No newline at end of file

\ No newline at end of file

> LayoutTests/inspector/elements-tests2.js:127
>  \ No newline at end of file

\ No newline at end of file
Comment 7 Alexander Pavlov (apavlov) 2010-12-24 08:42:09 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       LayoutTests/ChangeLog
        M       LayoutTests/http/tests/inspector/inspector-test2.js
        A       LayoutTests/inspector/elements-delete-inline-style-expected.txt
        A       LayoutTests/inspector/elements-delete-inline-style.html
        A       LayoutTests/inspector/elements-tests2.js
        M       LayoutTests/inspector/styles-add-blank-property.html
        M       WebCore/ChangeLog
        M       WebCore/inspector/InspectorCSSAgent.cpp
        M       WebCore/inspector/InspectorCSSAgent.h
        M       WebCore/inspector/InspectorDOMAgent.cpp
        M       WebCore/inspector/InspectorDOMAgent.h
        M       WebCore/inspector/InspectorStyleSheet.cpp
        M       WebCore/inspector/InspectorStyleSheet.h
        M       WebCore/inspector/front-end/ElementsTreeOutline.js
Committed r74634
Comment 8 Alexander Pavlov (apavlov) 2011-01-14 05:06:39 PST
*** Bug 35348 has been marked as a duplicate of this bug. ***