It supposed to navigate to the corresponding style in the Styles. Now it doesn't do anything. [Error] Assertion Failed: Styles panel is missing. computedStyleDetailsPanelShowProperty (ComputedStyleDetailsSidebarPanel.js:38) spreadsheetCSSStyleDeclarationEditorShowProperty (ComputedStyleDetailsPanel.js:85) spreadsheetStylePropertyShowProperty (ComputedStyleSection.js:249) (anonymous function) (SpreadsheetStyleProperty.js:228)
<rdar://problem/76854775>
This issue occurs when the Style panel is shown independently. When the Style panel is collapsed into the Details sidebar due to lack of space, this still works. Most likely the logic here doesn't know how to find the Style panel when it is part of a different Sidebar.
Created attachment 426452 [details] [Video] Doesn't select CSS property Note that when "Show independent styles sidebar" is unchecked, clicking on the go-to arrow open the Styles sidebar but doesn't focus on the CSS property. I haven't checked if this regressed in r268691 or not (if it did, it could be a separate bug/patch).
(In reply to Nikita Vasilyev from comment #3) > Note that when "Show independent styles sidebar" is unchecked, clicking on > the go-to arrow open the Styles sidebar but doesn't focus on the CSS > property. I haven't checked if this regressed in r268691 or not (if it did, > it could be a separate bug/patch). Just checked, the highlighting regressed somewhere else.
The second half of this behavior was regressed in r270134 due to a missed occurrence of `this._visible` (which should not have been accessed here at all as it was a private member of the superclass) in `WI.SpreadsheetRulesStyleDetailsPanel`.
Created attachment 426478 [details] Patch v1.0
Comment on attachment 426478 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=426478&action=review > Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsSidebarPanel.js:42 > + styleRulesSidebar = styleRulesSidebar.parentView; This kinda sorta feels like a layering violation :/ Is the issue here that the `WI.RulesStyleDetailsSidebarPanel` isn't kept inside the `sidebarPanels` of a `WI.MultiSidebar`? I feel like the `WI.MultiSidebar.prototype.get sidebarPanels` should return the list of _all_ sidebar panels across _all_ of its child/managed `WI.SingleSidebar`.
Comment on attachment 426478 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=426478&action=review >> Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsSidebarPanel.js:42 >> + styleRulesSidebar = styleRulesSidebar.parentView; > > This kinda sorta feels like a layering violation :/ > > Is the issue here that the `WI.RulesStyleDetailsSidebarPanel` isn't kept inside the `sidebarPanels` of a `WI.MultiSidebar`? I feel like the `WI.MultiSidebar.prototype.get sidebarPanels` should return the list of _all_ sidebar panels across _all_ of its child/managed `WI.SingleSidebar`. The issue is the `parentSidebar` is a SingleSidebar, which is a child of the MultiSidebar.
Comment on attachment 426478 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=426478&action=review > Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsSidebarPanel.js:43 > + } Can we just use `WI.detailsSidebar` then? Reaching into the `WI.View` hierarchy of a different class seems like a place where issues (likely undiscovered) could arise. It's also a bit presumptuous IMO to expect the `WI.RulesStyleDetailsSidebarPanel` to be in the same `WI.Sidebar` as this panel (as evidenced by this bug).
Comment on attachment 426478 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=426478&action=review >> Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsSidebarPanel.js:43 >> + } > > Can we just use `WI.detailsSidebar` then? Reaching into the `WI.View` hierarchy of a different class seems like a place where issues (likely undiscovered) could arise. It's also a bit presumptuous IMO to expect the `WI.RulesStyleDetailsSidebarPanel` to be in the same `WI.Sidebar` as this panel (as evidenced by this bug). That would certainly work. Sounds good to me.
Created attachment 426488 [details] Patch v1.1 - Revised approach
Comment on attachment 426488 [details] Patch v1.1 - Revised approach View in context: https://bugs.webkit.org/attachment.cgi?id=426488&action=review r=me, nice fix :) > Source/WebInspectorUI/ChangeLog:9 > + The Style panel will not always be part of the same sidebar as the Computed panel as of r268691, so we should > + look for the Style panel in WI.detailsSidebar, which will report all of the panels it manages across multiple NIT: "Styles panel" > Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsSidebarPanel.js:37 > - let styleRulesPanel = this.parentSidebar.sidebarPanels.find((panel) => panel instanceof WI.RulesStyleDetailsSidebarPanel); > + let styleRulesPanel = WI.detailsSidebar.sidebarPanels.find((panel) => panel instanceof WI.RulesStyleDetailsSidebarPanel); As a note for future readers, the reason why I don't see this as a layering violation is because this is the `WI.ComputedStyle*DETAILS*SidebarPanel`, so it's reasonable for a panel in the details sidebar to do things to the `WI.detailsSidebar`. Ideally we'd have a way to get to the related `WI.TabBrowser.prototype.get detailsSidebar` instead of assuming the global singleton (or doing as @Patrick Angle suggested offline and having a `get topLevelParentSidebar`), but in the interest of getting this fixed I think it's fine for now.
Created attachment 426492 [details] Patch v1.2 - Fix typos in changelog
Committed r276278 (236760@main): <https://commits.webkit.org/236760@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426492 [details].