Skip to content

Commit

Permalink
Fix broken URL parameters in new flame graph menus. (#813)
Browse files Browse the repository at this point in the history
Previously, we would pass an invalid 'f' URL parameter when switching
from the new Flamegraph view to another view if nothing was selected.
We now check for this case and avoid the bad behavior.

Also added a test for this behavior.
  • Loading branch information
ghemawat authored Oct 23, 2023
1 parent f7f687d commit ff6d637
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
14 changes: 11 additions & 3 deletions internal/driver/html/stacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,12 @@ function stackViewer(stacks, nodes) {
hiliter: (n, on) => { return hilite(n, on); },
current: () => {
let r = new Map();
for (let p of pivots) {
r.set(p, true);
if (pivots.length == 1 && pivots[0] == 0) {
// Not pivoting
} else {
for (let p of pivots) {
r.set(p, true);
}
}
return r;
}});
Expand Down Expand Up @@ -174,7 +178,11 @@ function stackViewer(stacks, nodes) {
function switchPivots(regexp) {
// Switch URL without hitting the server.
const url = new URL(document.URL);
url.searchParams.set('p', regexp);
if (regexp === '' || regexp === '^$') {
url.searchParams.delete('p'); // Not pivoting
} else {
url.searchParams.set('p', regexp);
}
history.pushState('', '', url.toString()); // Makes back-button work
matches = new Set();
search.value = '';
Expand Down
22 changes: 22 additions & 0 deletions internal/driver/testdata/testflame.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,28 @@ function TestFlame() {
checkWidth("F2", 100/100);
checkWidth("F3", 100/100);
});

test.run("NavigateWithoutPivot", function() {
// Clear pivot
boxMap.get("root").click();

// Trigger link update.
const btn = document.getElementById("graphbtn");
if (!btn) {
test.err("no graph button on page");
return;
}
const event = new Event("mouseenter");
btn.dispatchEvent(event);

// Check that URL does not contain a focus parameter.
test.log(btn.href);
const url = new URL(btn.href);
if (url.searchParams.has('f')) {
test.err("unexpected focus parameter in URL", btn.href);
}
});

return test.result;
}
TestFlame();

0 comments on commit ff6d637

Please sign in to comment.