Summary: | Web Inspector: Sources: Inconsistent selection in source tree when grouped by path | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Razvan Caliman <rcaliman> | ||||||||||||||
Component: | Web Inspector | Assignee: | Razvan Caliman <rcaliman> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Razvan Caliman
2021-05-03 11:12:34 PDT
Created attachment 427579 [details]
Video recording of bug
Created attachment 427583 [details]
Patch
Created attachment 427584 [details]
Patch
Comment on attachment 427584 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427583&action=review > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1050 > + let representedObject = resource.type === WI.Resource.Type.Document ? resource : null; Shouldn't this be `resource.parentFrame`? I think we want a different `representedObject` as otherwise the same `WI.Resource` would have two `WI.TreeElement` in the same `WI.TreeOutline`. > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1051 > + originTreeElement = new WI.OriginTreeElement(origin, representedObject, {hasChildren: true}); What does this show if `representedObject === null`? Is it just like any other (non-Images) folder in that it just shows some text? Created attachment 427586 [details]
Patch
(In reply to Devin Rousso from comment #4) > > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1050 > > + let representedObject = resource.type === WI.Resource.Type.Document ? resource : null; > > Shouldn't this be `resource.parentFrame`? I think we want a different > `representedObject` as otherwise the same `WI.Resource` would have two > `WI.TreeElement` in the same `WI.TreeOutline`. Yes, thanks for catching this! > > > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1051 > > + originTreeElement = new WI.OriginTreeElement(origin, representedObject, {hasChildren: true}); > > What does this show if `representedObject === null`? Is it just like any > other (non-Images) folder in that it just shows some text? Same behavior as with clicking on nested folders in Sources when grouped By Path: the previously rendered content stays in the main area until explicitly selecting another resource. Comment on attachment 427586 [details]
Patch
r=me
Comment on attachment 427586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427586&action=review > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1051 > + originTreeElement = new WI.OriginTreeElement(origin, representedObject, {hasChildren: true}); Should we assert in `WI.OriginTreeElement` so that this doesn't happen again? And what would happen if we use `null` here? Created attachment 427664 [details]
Patch
Carry over R+; Address code review feedback
Committed r276958 (237291@main): <https://commits.webkit.org/237291@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427664 [details]. |