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]
Comment on attachment 426465 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=426465&action=review
> + because`WI.DOMTreeOutline.selectedTreeElement`, not `WI.DOMTreeOutline.selectedDOMNode`, is ultimately used to
missing space between "... because `WI.DOMTreeOutline..."
> + 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")
> + 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]
Created attachment 426808 [details]
Comment on attachment 426808 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=426808&action=review
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`.
> + _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
> + 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 (this.contentBrowser.currentContentView?.representedObject === mainDOMTree)
Created attachment 426900 [details]
Comment on attachment 426900 [details]
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].