Avoid switching to the sources panel when attaching sourcemaps [devtools/devtools-frontend : main]
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Before I ask gemini for tests, WDYT overall?
Related details
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
Before I ask gemini for tests, WDYT overall?
Yeah, lgtm. Is this a regression caused by the UI eng vision migration?
Related details
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Yeah, lgtm. Is this a regression caused by the UI eng vision migration?
No, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Related details
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
No, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Somewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
Related details
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Somewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
During the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Related details
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Philip PfaffeSomewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
During the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Hmm, we might want to keep that though to a certain degree no? Especially if we pause first via `debugger` statement and then later resolve the location. Should we make this also dependent on the setting?
Related details
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Philip PfaffeSomewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
Simon ZündDuring the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Hmm, we might want to keep that though to a certain degree no? Especially if we pause first via `debugger` statement and then later resolve the location. Should we make this also dependent on the setting?
I don't think that should be setting-dependent. If the sources panel is not visible, switching to it when a sourcemap attaches feels like bad UX?
Related details
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Philip PfaffeSomewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
Simon ZündDuring the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Philip PfaffeHmm, we might want to keep that though to a certain degree no? Especially if we pause first via `debugger` statement and then later resolve the location. Should we make this also dependent on the setting?
I don't think that should be setting-dependent. If the sources panel is not visible, switching to it when a sourcemap attaches feels like bad UX?
Right, I think we should switch on initial pause, but not subsequently. At least for the same pause.
Related details
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Philip PfaffeSomewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
Simon ZündDuring the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Philip PfaffeHmm, we might want to keep that though to a certain degree no? Especially if we pause first via `debugger` statement and then later resolve the location. Should we make this also dependent on the setting?
Simon ZündI don't think that should be setting-dependent. If the sources panel is not visible, switching to it when a sourcemap attaches feels like bad UX?
Right, I think we should switch on initial pause, but not subsequently. At least for the same pause.
- Simon Zünd
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe voted and added 1 comment![]( "Open in Gerrit")
Votes added by Philip Pfaffe
| Auto-Submit | +1 |
Simon ZündBefore I ask gemini for tests, WDYT overall?
Philip PfaffeYeah, lgtm. Is this a regression caused by the UI eng vision migration?
Simon ZündNo, this predates the recent changes. I do find it a bit odd that updating the callstack triggers the reveal of the location. Is that working as intended?
Philip PfaffeSomewhat, the sequence is roughly -> Pause happens -> Paused details are set -> CallStackSidebar pane processes paused details and selects a frame -> location for the top frame is revealed.
Otherwise we might not reveal the paused location (i think).
Simon ZündDuring the normal pause process the location is revealed through the panel and not the sidebar. The bug is about location updates for sourcemaps. Sourcemap attaches -> CallStackSidebarPane is updated -> Frame is reselected -> location for frame is revealer.
Philip PfaffeHmm, we might want to keep that though to a certain degree no? Especially if we pause first via `debugger` statement and then later resolve the location. Should we make this also dependent on the setting?
Simon ZündI don't think that should be setting-dependent. If the sources panel is not visible, switching to it when a sourcemap attaches feels like bad UX?
Philip PfaffeRight, I think we should switch on initial pause, but not subsequently. At least for the same pause.
Agreed. That's already controlled by the setting.
Added tests, ptal!
Related details
- Simon Zünd
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd voted and added 1 comment![]( "Open in Gerrit")
Votes added by Simon Zünd
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Related details
- Philip Pfaffe
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe voted Commit-Queue+2![]( "Open in Gerrit")
- Simon Zünd
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devtools-frontend LUCI CQ (Gerrit)
Devtools-frontend LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Avoid switching to the sources panel when attaching sourcemaps
When sourcemaps attach, the call stack gets refreshed and if a frame is
selected, the corresponding UI location gets revealed. This causes some
disruption when a sourcemap attaches late and another panel is currently
open. That's also independent of the "Reveal sources panel on
breakpoints" setting. UI locations updating should never cause a switch
to the sources panel.
- M front_end/panels/sources/BUILD.gn
- A front_end/panels/sources/CallStackSidebarPane.test.ts
- M front_end/panels/sources/CallStackSidebarPane.ts
- M front_end/panels/sources/SourcesPanel.ts
Code-Review: +1 by Simon Zünd
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |