Created attachment 297049 [details] [HTML] Reduction Steps: 1. Open attached index.html 2. Inspect .box:before 3. Click on the [P] icon in the styles sidebar to comment out all CSS properties of the selected rule 4. Click on the [P] icon again to uncomment all CSS properties Expected: CSS properties get uncommented. Actual: CSS properties don't get uncommented.
Created attachment 297050 [details] [Animated GIF] Bug
Is this a regression?
Yes, this works as expected in macOS Sierra Safari but not in ToT WebKit. I didn't bisect yet to find where exactly it regressed.
<rdar://problem/29652688>
Created attachment 297480 [details] [Animated GIF] Re-enabling disabled CSS rules is broken Looks like this isn't just about pseudo elements — it can happen for any element.
(In reply to comment #5) > Created attachment 297480 [details] > [Animated GIF] Re-enabling disabled CSS rules is broken > > Looks like this isn't just about pseudo elements — it can happen for any > element. [Error] Assertion Failed _associateRelatedProperties (DOMNodeStyles.js:934) _updateStyleCascade (DOMNodeStyles.js:824) fetchedInlineStyles (DOMNodeStyles.js:146) fetchedInlineStyles _dispatchResponseToCallback (InspectorBackend.js:311) _dispatchResponse (InspectorBackend.js:281) dispatch (InspectorBackend.js:157) dispatchNextQueuedMessageFromBackend (MessageDispatcher.js:42)
(In reply to comment #6) > (In reply to comment #5) > > Created attachment 297480 [details] > > [Animated GIF] Re-enabling disabled CSS rules is broken > > > > Looks like this isn't just about pseudo elements — it can happen for any > > element. This is reproducible in macOS Sierra as well. > [Error] Assertion Failed > _associateRelatedProperties (DOMNodeStyles.js:934) > _updateStyleCascade (DOMNodeStyles.js:824) > fetchedInlineStyles (DOMNodeStyles.js:146) > fetchedInlineStyles > _dispatchResponseToCallback (InspectorBackend.js:311) > _dispatchResponse (InspectorBackend.js:281) > dispatch (InspectorBackend.js:157) > dispatchNextQueuedMessageFromBackend (MessageDispatcher.js:42) This error appears to be unrelated.
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > Created attachment 297480 [details] > > > [Animated GIF] Re-enabling disabled CSS rules is broken > > > > > > Looks like this isn't just about pseudo elements — it can happen for any > > > element. > > This is reproducible in macOS Sierra as well. Uncommenting inline rules turned out to be a separate issue from uncommenting pseudo-element rules. Bug 166297 - Web Inspector: Uncommenting CSS properties doesn't work for inline styles
Created attachment 297633 [details] Patch
It appears this never worked. The regression was in: Bug 166297 - Web Inspector: Uncommenting CSS properties doesn't work for inline styles.
Comment on attachment 297633 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297633&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:314 > + for (var property of properties) Nit: use `let`. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1294 > + var textMarker = property.__propertyTextMarker; Ditto. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1301 > + var range = textMarker.find(); Ditto.
(In reply to comment #10) > It appears this never worked. The regression was in: Bug 166297 - Web > Inspector: Uncommenting CSS properties doesn't work for inline styles. I don't understand your comment. A regression means the functionality has been decreased. If it never worked, then there was nothing to have regressed.
(In reply to comment #12) > (In reply to comment #10) > > It appears this never worked. The regression was in: Bug 166297 - Web > > Inspector: Uncommenting CSS properties doesn't work for inline styles. > > I don't understand your comment. A regression means the functionality has > been decreased. If it never worked, then there was nothing to have regressed. Uncommenting CSS rules of pseudo-elements by clicking [P] icon never worked. The other bug was a regression: Bug 166297 - Web Inspector: Uncommenting CSS properties doesn't work for inline styles.
Created attachment 297667 [details] Patch
Comment on attachment 297667 [details] Patch Clearing flags on attachment: 297667 Committed r210110: <http://trac.webkit.org/changeset/210110>
All reviewed patches have been landed. Closing bug.
Comment on attachment 297667 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297667&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1290 > _propertyCommentCheckboxChanged(event) > { > - var commentTextMarker = event.target.__commentTextMarker; > - console.assert(commentTextMarker); > - if (!commentTextMarker) > + this._uncommentProperty(event.target); > + } Here event.target is not a "property" it is a "checkbox". It will never have __propertyTextMarker. It will however have __commentTextMarker. This code is very ambiguous. I'll look at cleaning this up.
(In reply to comment #17) > Comment on attachment 297667 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=297667&action=review > > > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1290 > > _propertyCommentCheckboxChanged(event) > > { > > - var commentTextMarker = event.target.__commentTextMarker; > > - console.assert(commentTextMarker); > > - if (!commentTextMarker) > > + this._uncommentProperty(event.target); > > + } > > Here event.target is not a "property" it is a "checkbox". It will never have > __propertyTextMarker. It will however have __commentTextMarker. This code is > very ambiguous. I'll look at cleaning this up. My change broke uncommenting properties by clicking the checkbox :( Interestingly, commenting and uncommenting properties by pressing Cmd+/ still works, as it doesn't call _uncommentRange method.
I'm going to roll this out. It causes new regressions and doesn't seem to properly solve the original problem (it fails if there are any shorthand properties in the rule). I'll be addressing all commenting / uncommenting issues that appeared with the new CSS parser in a separate bug. There are a few approaches we can take that fixes all of these issues.
Re-opened since this is blocked by bug 166783
Duping to the single bug to handle CSS Toggling issues: *** This bug has been marked as a duplicate of bug 166786 ***