When you have a node selected in the inspector, the computed style should change as the node changes. At worst, clicking again on the node should update the values. Currently, you have to select away from the node, then reselect it to get updated values. This makes it hard to debug whether something is changing.
Saw this in Chrome trunk. Not sure if it happens in Safari.
It tries to update, but dies trying. Could you please share the url you are seeing it on? Or open inspector on inspector and dump console errors here please!
I saw it on the chrome NTP.
Created attachment 65443 [details] before Before and after shots of updating the style at http://plexode.com/eval3/#ht=%3Cdiv%20id%3Dfoo%3Efoo%3C%2Fdiv%3E&ohh=1&ohj=1&jt=foo.style.border%20%3D%20%221px%20solid%22&ojh=0&ojj=1&ms=100&oth=0&otj=0&cex=1
Created attachment 65444 [details] after
Investigation note: inspector does not detect changes to the style of the currently selected node when they are done programmatically by someone other than inspector. We should detect style recalculation on the selected node and update inspector.
Created attachment 80194 [details] [PATCH] Suggested solution
Comment on attachment 80194 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=80194&action=review > Source/WebCore/css/CSSMutableStyleDeclaration.cpp:590 > + setNeedsStyleRecalc(true); I'd add #if ENABLED(INSPECTOR) here and do all the work here instead of changing the method signature. > Source/WebCore/inspector/InspectorStyleSheet.cpp:1197 > +inline CSSStyleDeclaration* InspectorStyleSheetForInlineStyle::inlineStyle() const Did this help? Any stats? > Source/WebCore/inspector/front-end/ElementsPanel.js:465 > + if (WebInspector.isEditingAnyField()) Is this synchronous? > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1772 > + WebInspector.panels.elements.modifyingStyle = true; You should define this on sidebar.
Comment on attachment 80194 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=80194&action=review > Source/WebCore/css/CSSMutableStyleDeclaration.cpp:464 > +void CSSMutableStyleDeclaration::setNeedsStyleRecalc(bool viaStyleAttribute) Might be cleaner to reuse the enum for the node setNeedsStyleRecalc. Then you could use InlineStyleChange etc.
(In reply to comment #8) > (From update of attachment 80194 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80194&action=review > > > Source/WebCore/css/CSSMutableStyleDeclaration.cpp:590 > > + setNeedsStyleRecalc(true); > > I'd add #if ENABLED(INSPECTOR) here and do all the work here instead of changing the method signature. The point is that the notification should occur when the value is _false_, i.e. in the remaining 7 cases. I will follow up with dhyatt's suggestion. > > Source/WebCore/inspector/InspectorStyleSheet.cpp:1197 > > +inline CSSStyleDeclaration* InspectorStyleSheetForInlineStyle::inlineStyle() const > > Did this help? Any stats? Around 0.8-1.0% - perhaps is not worth it. > > Source/WebCore/inspector/front-end/ElementsPanel.js:465 > > + if (WebInspector.isEditingAnyField()) > > Is this synchronous? Yes, this is synchronous. It just checks a flag set by WebInspector.startEditing(). But for this, every time we increment/decrement a numeric property value in the Styles pane, the style change would eventually reach this method and update the Styles pane itself, thereby killing the active editor. This check only forbids "killing" the active property editor, no other editing use cases are affected, in the frontend or backend. > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1772 > > + WebInspector.panels.elements.modifyingStyle = true; > > You should define this on sidebar. Done.
Created attachment 81199 [details] [PATCH] Comments addressed
Comment on attachment 81199 [details] [PATCH] Comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=81199&action=review > Source/WebCore/css/CSSMutableStyleDeclaration.cpp:474 > + if (styleAttributeChange == FullStyleChange && m_node->document()) I think there has been misunderstanding: your boolean values had different meaning than the ones in StyleChangeType. What you are fighting was dupe in the didModifyAttr notification. You should either find out what negative implications this dupe has or come up with a separate InspectorInstrumentation::inlineStyleNeedsRecalc(m_node) signal that you would fire here unconditionally. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1770 > + WebInspector.panels.elements.sidebarPanes.styles.isModifyingStyle = true; Please pass styles pane into section instead.
*** Bug 51118 has been marked as a duplicate of this bug. ***
@pfeldman: as a lot of complications have risen from the delayed notification approach, I suggest landing the current solution as-is (with proper fixes applied) and think about something different afterwards, as users are asking for this issue to be fixed.
Created attachment 82797 [details] [PATCH] The timer-based attribute revalidation approach
Comment on attachment 82797 [details] [PATCH] The timer-based attribute revalidation approach View in context: https://bugs.webkit.org/attachment.cgi?id=82797&action=review > LayoutTests/inspector/styles/styles-update-from-js.html:21 > + InspectorTest.evaluateInPage("modifyStyleAttribute()", f); You should not rely upon pending dispatches - use sniffers instead. > LayoutTests/inspector/styles/styles-update-from-js.html:28 > + InspectorTest.runAfterPendingDispatches(setParsedAttribute); ditto > LayoutTests/inspector/styles/styles-update-from-js.html:41 > + InspectorTest.runAfterPendingDispatches(done); ditto > LayoutTests/inspector/styles/styles-update-from-js.html:70 > + var innerMapping = WebInspector.domAgent._idToDOMNode; treeOutline.findTreeElement(InspectorTest.expandedNodeWithId("container")) > LayoutTests/inspector/styles/styles-update-from-js.html:80 > + InspectorTest.expandElementsTree(selectContainerElementContinuation); 1) Tests now flow top->bottom. 2) No need to expand tree, start with selectNode > LayoutTests/inspector/styles/styles-update-from-js.html:83 > +function modifyStyleAttribute() test functions go before test() > LayoutTests/inspector/styles/styles-update-from-js.html:110 > +<div id="other"> Remove this div. > Source/WebCore/css/CSSMutableStyleDeclaration.cpp:475 > + InspectorInstrumentation::didInvalidateStyleAttr(m_node->document(), static_cast<StyledElement*>(m_node)); Simply pass node here, do all the casts and gets in the instrumentation code. > Source/WebCore/dom/StyledElement.cpp:255 > + setNeedsStyleRecalc(InlineStyleChange); Please revert or ask for Dave's blessing. > Source/WebCore/inspector/InspectorDOMAgent.cpp:238 > + , m_revalidateStyleAttrTask(this) You should only create this timer when needed. > Source/WebCore/inspector/InspectorDOMAgent.h:148 > + class RevalidateStyleAttrTask { No abbreviations in WebCore. You should also do a forward declaration here with private implementation in cpp. > Source/WebCore/inspector/InspectorStyleSheet.cpp:1202 > +const String& InspectorStyleSheetForInlineStyle::styleTextFromElement() const elementStyleText ?
Created attachment 82827 [details] [PATCH] Comments addressed All comments have been addressed in this patch
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/inspector/styles/styles-update-from-js-expected.txt A LayoutTests/inspector/styles/styles-update-from-js.html M Source/WebCore/ChangeLog M Source/WebCore/css/CSSMutableStyleDeclaration.cpp M Source/WebCore/inspector/InspectorDOMAgent.cpp M Source/WebCore/inspector/InspectorDOMAgent.h M Source/WebCore/inspector/InspectorInstrumentation.cpp M Source/WebCore/inspector/InspectorInstrumentation.h M Source/WebCore/inspector/InspectorStyleSheet.cpp M Source/WebCore/inspector/InspectorStyleSheet.h M Source/WebCore/inspector/front-end/ElementsPanel.js M Source/WebCore/inspector/front-end/StylesSidebarPane.js Committed r79009