<rdar://73470211> Occurs when starting Web Inspector on any tab other than Elements. Steps to reproduce: 1. Ensure Timelines Tab will open on Inspector launch (select Timelines, close inspector) 2. Open Inspector, timelines tab should be showing. 3. Switch to Elements Tab => Sidebar Pane toggle is disabled - Icon Tooltip: "Hide the details sidebar" => Elements Tab does not have an actual selection. - Breadcrumbs are not shown in the navigation bar. Workaround: select a new DOM node and the sidebars come back with the icon toggled on.
Created attachment 426465 [details] Patch
Comment on attachment 426465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426465&action=review > Source/WebInspectorUI/ChangeLog:10 > + because`WI.DOMTreeOutline.selectedTreeElement`, not `WI.DOMTreeOutline.selectedDOMNode`, is ultimately used to missing space between "... because `WI.DOMTreeOutline..." > Source/WebInspectorUI/ChangeLog:11 > + identify the current represented object when rendering the Elements tab details sidebar panels. Style: we capitalize "Tab" when used with a tab name (e.g. "Elements Tab" vs "a tab") > Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.js:521 > + this.dispatchEventToListeners(WI.ContentView.Event.SelectionPathComponentsDidChange); I'm slightly confused. `WI.DOMTreeContentView.prototype._selectedNodeDidChange` should already be called in response to `WI.DOMTreeContentView.prototype._domTreeSelectionDidChange`. `WI.DOMTreeContentView.prototype._selectedNodeDidChange` is the event handler for `WI.DOMTreeOutline.Event.SelectedNodeChanged`, which is dispatched inside `WI.DOMTreeOutline.prototype._selectedNodeChanged`, which is called by `WI.DOMTreeOutline.prototype.selectDOMNode`. So why is it that `WI.DOMTreeOutline.Event.SelectedNodeChanged` isn't getting dispatched? Is the `_selectedDOMNode` already somehow set?
Created attachment 426548 [details] Patch Update Changelog
Created attachment 426808 [details] Patch
Comment on attachment 426808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426808&action=review > Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:117 > } Please still call `super.detached()` at the end just in case one of the inherited classes decides to add a `detached` in the future so we don't subtly miss out on that behavior. Also, before this change we would've used `super.detached` by default, so we should preserve that. As a general rule of thumb we almost always (and you need a good reason not to) want to call `super` for any methods inherited from `WI.View`. > Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:143 > + _showDOMTreeContentViewIfNeeded() I think we can get rid of `_showDOMTreeContentView` while we're at it, as all the callsites are being called either - in response to a main frame/resource change, so we should have a new `WI.DOMTree` anyways - when there is currently no content view being shown, meaning we should show one with whatever the main frame main resource `WI.DOMTree` is right now > Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:147 > + if (mainDOMTree && this.contentBrowser.currentContentView?.representedObject !== mainDOMTree); > + this.contentBrowser.showContentViewForRepresentedObject(mainDOMTree); NIT: I'd maybe turn this into an series of early-return so that it's clearer what the cases are for the "if needed" part ``` let mainDOMTree = WI.networkManager.mainFrame?.domTree; if (!mainDOMTree) return; if (this.contentBrowser.currentContentView?.representedObject === mainDOMTree) return; this.contentBrowser.showContentViewForRepresentedObject(mainDOMTree); ```
Created attachment 426900 [details] Patch
Comment on attachment 426900 [details] Patch r=me
Committed r276587 (237023@main): <https://commits.webkit.org/237023@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426900 [details].