Bug 51478 - Web Inspector: Inline HTML style property out of sync with element.style in Sidebar
Summary: Web Inspector: Inline HTML style property out of sync with element.style in S...
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:
: 35348 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-22 09:59 PST by Joseph Pecoraro
Modified: 2011-01-14 05:06 PST (History)
10 users (show)

See Also:


Attachments
[PATCH] Suggested fix (13.43 KB, patch)
2010-12-24 02:37 PST, Alexander Pavlov (apavlov)
yurys: review+
Details | Formatted Diff | Diff
[PATCH] Comments addressed (19.99 KB, patch)
2010-12-24 08:23 PST, Alexander Pavlov (apavlov)
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***