From 58f208134c529b8b84c8a51a73e3170162e46dcd Mon Sep 17 00:00:00 2001 From: fatadel Date: Mon, 16 Feb 2026 16:35:12 +0100 Subject: [PATCH 01/11] persist selected marker --- src/actions/receive-profile.ts | 14 +++++++++++++- src/app-logic/url-handling.ts | 23 ++++++++++++++++++++++- src/reducers/url-state.ts | 20 ++++++++++++++++++++ src/selectors/url-state.ts | 3 +++ src/types/state.ts | 5 +++++ 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/actions/receive-profile.ts b/src/actions/receive-profile.ts index 76909edbdb..7ed898cf6e 100644 --- a/src/actions/receive-profile.ts +++ b/src/actions/receive-profile.ts @@ -41,6 +41,7 @@ import { import { getSelectedTab, getTabFilter, + getSelectedMarkers, } from 'firefox-profiler/selectors/url-state'; import { getTabToThreadIndexesMap, @@ -64,7 +65,7 @@ import { computeDefaultHiddenTracks, getVisibleThreads, } from 'firefox-profiler/profile-logic/tracks'; -import { setDataSource } from './profile-view'; +import { setDataSource, changeSelectedMarker } from './profile-view'; import { fatalError } from './errors'; import { batchLoadDataUrlIcons } from './icons'; import { GOOGLE_STORAGE_BUCKET } from 'firefox-profiler/app-logic/constants'; @@ -371,6 +372,17 @@ export function finalizeFullProfileView( ...hiddenTracks, }); }); + + // Initialize selected markers from URL state if present + if (hasUrlInfo) { + const selectedMarkers = getSelectedMarkers(getState()); + // Dispatch marker selection for each thread that has a marker in URL + for (const [threadsKey, markerIndex] of Object.entries(selectedMarkers)) { + if (markerIndex !== null) { + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + } + } + } }; } diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 3c427a9aa0..8d1d3d172d 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -40,6 +40,7 @@ import type { NativeSymbolInfo, Transform, IndexIntoFrameTable, + MarkerIndex, } from 'firefox-profiler/types'; import { decodeUintArrayFromUrlComponent, @@ -49,7 +50,7 @@ import { import { tabSlugs } from '../app-logic/tabs-handling'; import { StringTable } from 'firefox-profiler/utils/string-table'; -export const CURRENT_URL_VERSION = 13; +export const CURRENT_URL_VERSION = 14; /** * This static piece of state might look like an anti-pattern, but it's a relatively @@ -174,6 +175,7 @@ type BaseQuery = { timelineType: string; sourceViewIndex: number; assemblyView: string; + marker: MarkerIndex; // MarkerIndex for the selected thread, e.g. "42" }; type CallTreeQuery = BaseQuery & { @@ -292,6 +294,11 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { selectedThreads === null ? undefined : encodeUintSetForUrlComponent(selectedThreads), + marker: + selectedThreadsKey && + urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null + ? urlState.profileSpecific.selectedMarkers[selectedThreadsKey] + : undefined, file: urlState.pathInZipFile || undefined, profiles: urlState.profilesToCompare || undefined, v: CURRENT_URL_VERSION, @@ -499,6 +506,15 @@ export function stateFromLocation( transforms[selectedThreadsKey] = parseTransforms(query.transforms); } + // Parse the selected marker for the current thread + const selectedMarkers: { [key: string]: MarkerIndex | null } = {}; + if (selectedThreadsKey && query.marker) { + const markerIndex = Number(query.marker); + if (!isNaN(markerIndex)) { + selectedMarkers[selectedThreadsKey] = markerIndex; + } + } + // tabID is used for the tab selector that we have in our full view. let tabID = null; if (query.tabID && Number.isInteger(Number(query.tabID))) { @@ -587,6 +603,7 @@ export function stateFromLocation( legacyHiddenThreads: query.hiddenThreads ? query.hiddenThreads.split('-').map((index) => Number(index)) : null, + selectedMarkers, }, }; } @@ -1196,6 +1213,10 @@ const _upgraders: { [13]: (_) => { // just added the focus-self transform }, + [14]: (_) => { + // Added marker parameter for persisting highlighted markers in URLs. + // This is backward compatible as the marker parameter is optional. + }, }; for (let destVersion = 1; destVersion <= CURRENT_URL_VERSION; destVersion++) { diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 8903ea34b2..3e3f5b6379 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -23,6 +23,7 @@ import type { AssemblyViewState, IsOpenPerPanelState, TabID, + SelectedMarkersPerThread, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -677,6 +678,24 @@ const symbolServerUrl: Reducer = (state = null) => { return state; }; +const selectedMarkers: Reducer = ( + state = {}, + action +): SelectedMarkersPerThread => { + switch (action.type) { + case 'CHANGE_SELECTED_MARKER': { + const { threadsKey, selectedMarker } = action; + // Store the selected marker for this specific threadsKey + return { + ...state, + [threadsKey]: selectedMarker, + }; + } + default: + return state; + } +}; + /** * These values are specific to an individual profile. */ @@ -703,6 +722,7 @@ const profileSpecific = combineReducers({ localTrackOrderChangedPids, showJsTracerSummary, tabFilter, + selectedMarkers, // The timeline tracks used to be hidden and sorted by thread indexes, rather than // track indexes. The only way to migrate this information to tracks-based data is to // first retrieve the profile, so they can't be upgraded by the normal url upgrading diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 3045882da5..3111c51021 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -31,6 +31,7 @@ import type { NativeSymbolInfo, TabID, IndexIntoSourceTable, + SelectedMarkersPerThread, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -167,6 +168,8 @@ export const getTabFilter: Selector = (state) => getProfileSpecificState(state).tabFilter; export const hasTabFilter: Selector = (state) => getTabFilter(state) !== null; +export const getSelectedMarkers: Selector = (state) => + getProfileSpecificState(state).selectedMarkers; /** * This selector does a simple lookup in the set of hidden tracks for a PID, and ensures diff --git a/src/types/state.ts b/src/types/state.ts index 792a1ded08..7cf15967bc 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -84,6 +84,10 @@ export type MarkerReference = { readonly markerIndex: MarkerIndex; }; +export type SelectedMarkersPerThread = { + [key: ThreadsKey]: MarkerIndex | null; +}; + /** * Profile view state */ @@ -373,6 +377,7 @@ export type ProfileSpecificUrlState = { tabFilter: TabID | null; legacyThreadOrder: ThreadIndex[] | null; legacyHiddenThreads: ThreadIndex[] | null; + selectedMarkers: SelectedMarkersPerThread; }; export type UrlState = { From b85d3306cb30e464a0b003a4d4cd21dd97bbca11 Mon Sep 17 00:00:00 2001 From: fatadel Date: Mon, 16 Feb 2026 19:01:16 +0100 Subject: [PATCH 02/11] make selected marker id part of MarkersQuery --- src/app-logic/url-handling.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 8d1d3d172d..1ec5761733 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -175,7 +175,6 @@ type BaseQuery = { timelineType: string; sourceViewIndex: number; assemblyView: string; - marker: MarkerIndex; // MarkerIndex for the selected thread, e.g. "42" }; type CallTreeQuery = BaseQuery & { @@ -186,6 +185,7 @@ type CallTreeQuery = BaseQuery & { type MarkersQuery = BaseQuery & { markerSearch: string; // "DOMEvent" + marker?: MarkerIndex; // Selected marker index for the current thread, e.g. 42 }; type NetworkQuery = BaseQuery & { @@ -220,6 +220,7 @@ type Query = BaseQuery & { // Markers specific markerSearch?: string; + marker?: MarkerIndex; // Network specific networkSearch?: string; @@ -294,11 +295,6 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { selectedThreads === null ? undefined : encodeUintSetForUrlComponent(selectedThreads), - marker: - selectedThreadsKey && - urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null - ? urlState.profileSpecific.selectedMarkers[selectedThreadsKey] - : undefined, file: urlState.pathInZipFile || undefined, profiles: urlState.profilesToCompare || undefined, v: CURRENT_URL_VERSION, @@ -374,6 +370,11 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { query = baseQuery as MarkersQueryShape; query.markerSearch = urlState.profileSpecific.markersSearchString || undefined; + query.marker = + selectedThreadsKey !== null && + urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null + ? urlState.profileSpecific.selectedMarkers[selectedThreadsKey] + : undefined; break; case 'network-chart': query = baseQuery as NetworkQueryShape; @@ -508,7 +509,11 @@ export function stateFromLocation( // Parse the selected marker for the current thread const selectedMarkers: { [key: string]: MarkerIndex | null } = {}; - if (selectedThreadsKey && query.marker) { + if ( + selectedThreadsKey !== null && + query.marker !== undefined && + query.marker !== null + ) { const markerIndex = Number(query.marker); if (!isNaN(markerIndex)) { selectedMarkers[selectedThreadsKey] = markerIndex; From 0fc3136578aea9452040841823051206e1af11c7 Mon Sep 17 00:00:00 2001 From: fatadel Date: Mon, 16 Feb 2026 19:26:10 +0100 Subject: [PATCH 03/11] add tests --- src/test/url-handling.test.ts | 262 +++++++++++++++++++++++++++++++++- 1 file changed, 261 insertions(+), 1 deletion(-) diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index 665069f8f1..99afc09fce 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -20,6 +20,7 @@ import { closeBottomBox, changeShowUserTimings, changeStackChartSameWidths, + changeSelectedMarker, } from '../actions/profile-view'; import { changeSelectedTab, changeProfilesToCompare } from '../actions/app'; import { @@ -47,7 +48,10 @@ import { getHumanReadableTracks, getProfileWithNiceTracks, } from './fixtures/profiles/tracks'; -import { getProfileFromTextSamples } from './fixtures/profiles/processed-profile'; +import { + getProfileFromTextSamples, + getProfileWithMarkers, +} from './fixtures/profiles/processed-profile'; import { selectedThreadSelectors } from '../selectors/per-thread'; import { encodeUintArrayForUrlComponent, @@ -1987,3 +1991,259 @@ describe('tab selector', function () { expect(urlStateSelectors.hasTabFilter(getState())).toEqual(true); }); }); + +describe('selectedMarker', function () { + function setup(search = '', pathname?: string) { + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); + const settings: StoreUrlSettings = { search }; + if (pathname) { + settings.pathname = pathname; + } + return _getStoreWithURL(settings, profile); + } + + it('can serialize a selected marker to the URL on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('can unserialize a selected marker from the URL on marker-chart tab', () => { + const markerIndex = 2; + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); + const { getState } = _getStoreWithURL( + { + pathname: + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/', + search: `?thread=0&marker=${markerIndex}`, + }, + profile + ); + + expect( + selectedThreadSelectors.getViewOptions(getState()).selectedMarker + ).toEqual(markerIndex); + }); + + it('handles marker index 0 correctly on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 0; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + expect(queryString).not.toContain('marker=undefined'); + }); + + it('does not include marker in URL when switching to a thread without a selected marker on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey0 = getThreadsKey(new Set([0])); + + // Select a marker on thread 0 + dispatch(changeSelectedMarker(threadsKey0, markerIndex)); + expect(getQueryStringFromState(getState())).toContain( + `marker=${markerIndex}` + ); + + // Switch to thread 1 (no marker selected) + dispatch(changeSelectedThreads(new Set([1]))); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('preserves selected marker in internal state when switching back to the thread on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey0 = getThreadsKey(new Set([0])); + + // Select a marker on thread 0 + dispatch(changeSelectedMarker(threadsKey0, markerIndex)); + + // Switch to thread 1 + dispatch(changeSelectedThreads(new Set([1]))); + + // Marker should not be in URL for thread 1 + expect(getQueryStringFromState(getState())).not.toContain('marker='); + + // Switch back to thread 0 + dispatch(changeSelectedThreads(new Set([0]))); + + // The marker should reappear in the URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('handles null marker index correctly on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker first + dispatch(changeSelectedMarker(threadsKey, 1)); + expect(getQueryStringFromState(getState())).toContain('marker=1'); + + // Deselect the marker + dispatch(changeSelectedMarker(threadsKey, null)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('survives a URL round-trip on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 2; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Get the URL + const urlState = urlStateSelectors.getUrlState(getState()); + const url = urlFromState(urlState); + + // URL should contain the marker + expect(url).toContain(`marker=${markerIndex}`); + + // Parse the URL back + const newUrlState = stateFromLocation( + new URL(url, 'https://profiler.firefox.com') + ); + + // The marker should be in the parsed URL state + expect(newUrlState.profileSpecific.selectedMarkers[threadsKey]).toEqual( + markerIndex + ); + }); + + it('does NOT include marker in URL on calltree tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/calltree/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to calltree since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Select a marker on calltree tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker should NOT be in URL for calltree tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('does NOT include marker in URL on flame-graph tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/flame-graph/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to flame-graph since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'flame-graph' }); + + // Select a marker on flame-graph tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker should NOT be in URL for flame-graph tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('includes marker in URL on marker-table tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-table/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker on marker-table tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker SHOULD be in URL for marker-table tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('removes marker from URL when switching from marker-chart to calltree', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker on marker-chart + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + expect(getQueryStringFromState(getState())).toContain( + `marker=${markerIndex}` + ); + + // Switch to calltree tab + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Marker should be removed from URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('adds marker to URL when switching from calltree to marker-chart', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/calltree/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to calltree since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Select a marker on calltree (won't be in URL) + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + expect(getQueryStringFromState(getState())).not.toContain('marker='); + + // Switch to marker-chart tab + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'marker-chart' }); + + // Marker should now appear in URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); +}); From ea8a8170acc940da819c6ecbb9b15d2f7d857af2 Mon Sep 17 00:00:00 2001 From: fatadel Date: Tue, 17 Feb 2026 13:34:37 +0100 Subject: [PATCH 04/11] show sticky tooltip for selected marker on load --- src/components/marker-chart/Canvas.tsx | 98 ++++++++++++++++++++++++++ src/components/shared/chart/Canvas.tsx | 53 ++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/src/components/marker-chart/Canvas.tsx b/src/components/marker-chart/Canvas.tsx index 3f49c0d3e6..6bf0dfb798 100644 --- a/src/components/marker-chart/Canvas.tsx +++ b/src/components/marker-chart/Canvas.tsx @@ -957,8 +957,104 @@ class MarkerChartCanvasImpl extends React.PureComponent { ); }; + /** + * Calculate the canvas position for a marker's tooltip based on the marker's index. + * The method is used when the selected marker comes from the URL and does NOT + * necessarily correspond to what's being hovered at the moment. + * + * @param markerIndex - The marker's index in the original thread's marker table + * @returns Canvas-relative coordinates {offsetX, offsetY} or null if marker not visible + */ + getTooltipPosition = ( + markerIndex: MarkerIndex + ): { offsetX: CssPixels; offsetY: CssPixels } | null => { + const { + rangeStart, + rangeEnd, + markerTimingAndBuckets, + rowHeight, + marginLeft, + marginRight, + viewport: { containerWidth, viewportLeft, viewportRight, viewportTop }, + } = this.props; + + // Step 1: Find which row this marker is displayed in + // markerIndexToTimingRow is a map: markerIndex -> rowIndex + const markerIndexToTimingRow = this._getMarkerIndexToTimingRow( + markerTimingAndBuckets + ); + const rowIndex = markerIndexToTimingRow[markerIndex]; + + if (rowIndex === undefined) { + // Marker is not in any visible row (might be filtered out) + return null; + } + + // Step 2: Get the timing data for all markers in this row + // markerTiming contains arrays of data for all markers in this row + const markerTiming = markerTimingAndBuckets[rowIndex]; + if (!markerTiming || typeof markerTiming === 'string') { + // Row is empty or is a bucket label (string), not actual marker data + return null; + } + + // Step 3: Find the position of our specific marker within this row's data + // markerTiming.index[] contains the original marker indexes for all markers in this row + // We need to find which position (markerTimingIndex) corresponds to our markerIndex + // so we can look up its start/end times in the parallel arrays + let markerTimingIndex = -1; + for (let i = 0; i < markerTiming.length; i++) { + if (markerTiming.index[i] === markerIndex) { + markerTimingIndex = i; + break; + } + } + + if (markerTimingIndex === -1) { + // Marker not found in this row's data (shouldn't happen, but handle gracefully) + return null; + } + + // Step 4: Calculate horizontal (X) position + // Get the marker's start and end timestamps from the parallel arrays + const startTimestamp = markerTiming.start[markerTimingIndex]; + const endTimestamp = markerTiming.end[markerTimingIndex]; + + // Convert absolute timestamps to relative positions (0.0 to 1.0 of the full range) + const markerContainerWidth = containerWidth - marginLeft - marginRight; + const rangeLength: Milliseconds = rangeEnd - rangeStart; + const viewportLength: UnitIntervalOfProfileRange = + viewportRight - viewportLeft; + const startTime: UnitIntervalOfProfileRange = + (startTimestamp - rangeStart) / rangeLength; + const endTime: UnitIntervalOfProfileRange = + (endTimestamp - rangeStart) / rangeLength; + + // Calculate pixel position: map the time range to the visible viewport + const x: CssPixels = + ((startTime - viewportLeft) * markerContainerWidth) / viewportLength + + marginLeft; + const w: CssPixels = + ((endTime - startTime) * markerContainerWidth) / viewportLength; + + // For instant markers, use the center. For interval markers, use a position near the start + + // For instant markers (start === end), use the center point + // For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller) + const isInstantMarker = startTimestamp === endTimestamp; + const offsetX = isInstantMarker ? x : x + Math.min(w / 3, 30); + + // Step 5: Calculate vertical (Y) position + // Place the tooltip at the top of the marker's row, with a small offset + const offsetY: CssPixels = rowIndex * rowHeight - viewportTop + 5; + + // Return canvas-relative coordinates (should be converted to page coordinates by caller) + return { offsetX, offsetY }; + }; + override render() { const { containerWidth, containerHeight, isDragging } = this.props.viewport; + const { selectedMarkerIndex } = this.props; return ( { onMouseMove={this.onMouseMove} onMouseLeave={this.onMouseLeave} stickyTooltips={true} + selectedItem={selectedMarkerIndex} + getTooltipPosition={this.getTooltipPosition} /> ); } diff --git a/src/components/shared/chart/Canvas.tsx b/src/components/shared/chart/Canvas.tsx index c181d59158..24c3334fc7 100644 --- a/src/components/shared/chart/Canvas.tsx +++ b/src/components/shared/chart/Canvas.tsx @@ -33,6 +33,12 @@ type Props = { readonly onMouseLeave?: (e: { nativeEvent: MouseEvent }) => unknown; // Defaults to false. Set to true if the chart should persist the tooltips on click. readonly stickyTooltips?: boolean; + // Selected item coming from URL or other non-click mechanism. + readonly selectedItem?: Item | null; + // Method to compute tooltip position for the selected item above. + readonly getTooltipPosition?: ( + item: Item + ) => { offsetX: CssPixels; offsetY: CssPixels } | null; }; // The naming of the X and Y coordinates here correspond to the ones @@ -359,6 +365,43 @@ export class ChartCanvas extends React.Component< this._canvas = canvas; }; + _syncSelectedItemFromProp = (selectedItem: Item | null) => { + if (selectedItem === null) { + this.setState({ selectedItem: null }); + return; + } + + if (!this.props.getTooltipPosition || !this._canvas) { + return; + } + + const tooltipPosition = this.props.getTooltipPosition(selectedItem); + if (!tooltipPosition) { + // Can't get position, but still set the selectedItem + this.setState({ selectedItem }); + return; + } + + const canvasRect = this._canvas.getBoundingClientRect(); + const pageX = canvasRect.left + window.scrollX + tooltipPosition.offsetX; + const pageY = canvasRect.top + window.scrollY + tooltipPosition.offsetY; + this.setState({ + selectedItem, + pageX, + pageY, + }); + }; + + override componentDidMount() { + // Initialize selectedItem from props on mount if provided + // Use requestAnimationFrame to ensure the canvas is fully laid out + if (this.props.selectedItem !== undefined) { + window.requestAnimationFrame(() => { + this._syncSelectedItemFromProp(this.props.selectedItem!); + }); + } + } + override UNSAFE_componentWillReceiveProps() { // It is possible that the data backing the chart has been // changed, for instance after symbolication. Clear the @@ -377,6 +420,16 @@ export class ChartCanvas extends React.Component< override componentDidUpdate(prevProps: Props, prevState: State) { if (prevProps !== this.props) { + // Sync selectedItem state with selectedItem prop if it changed + // and the state doesn't already have this item selected. + if ( + this.props.selectedItem !== undefined && + this.props.selectedItem !== prevProps.selectedItem && + !hoveredItemsAreEqual(this.state.selectedItem, this.props.selectedItem) + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + if ( this.state.selectedItem !== null && prevState.selectedItem === this.state.selectedItem From e227fa2214c6ba0bc014e773a873e752dd6abb43 Mon Sep 17 00:00:00 2001 From: fatadel Date: Tue, 17 Feb 2026 14:37:43 +0100 Subject: [PATCH 05/11] sync selected marker with browser history --- src/reducers/profile-view.ts | 49 ++++++++++++++++++++++++++-------- src/test/url-handling.test.ts | 50 +++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index 26465510fd..df8d1ae0c7 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -403,24 +403,44 @@ const viewOptionsPerThread: Reducer = ( }); } case 'UPDATE_URL_STATE': { - // When the URL state changes (e.g., via browser back button), check if the - // transform stack has been popped for each thread. If so, reset the stored paths - // because they may reference call nodes that only exist in a transformed tree. - // See: https://github.com/firefox-devtools/profiler/issues/5689 + // When the URL state changes (e.g., via browser back button): + // 1. Check if the transform stack has been popped for each thread. + // If so, reset the stored paths, because they may reference call nodes + // that only exist in a transformed tree. + // See: https://github.com/firefox-devtools/profiler/issues/5689. + // 2. Sync selected marker with URL state. + if (!action.newUrlState) { return state; } - const { transforms } = action.newUrlState.profileSpecific; + const { transforms, selectedMarkers } = + action.newUrlState.profileSpecific; return objectMap(state, (viewOptions, threadsKey) => { const transformStack = transforms[threadsKey] || []; const newTransformCount = transformStack.length; const oldTransformCount = viewOptions.lastSeenTransformCount; - // If transform count changed, reset the paths. - if (newTransformCount < oldTransformCount) { - return { - ...viewOptions, + // Get the selected marker from URL state for this thread + const urlSelectedMarker = selectedMarkers[threadsKey] ?? null; + const currentSelectedMarker = viewOptions.selectedMarker; + + // Check if we need to update anything + const transformCountChanged = newTransformCount < oldTransformCount; + const markerChanged = urlSelectedMarker !== currentSelectedMarker; + + if (!transformCountChanged && !markerChanged) { + // No change needed + return viewOptions; + } + + // Build the updated view options + let updatedOptions = { ...viewOptions }; + + // If transform count changed, reset the paths + if (transformCountChanged) { + updatedOptions = { + ...updatedOptions, selectedNonInvertedCallNodePath: [], selectedInvertedCallNodePath: [], expandedNonInvertedCallNodePaths: new PathSet(), @@ -429,8 +449,15 @@ const viewOptionsPerThread: Reducer = ( }; } - // No change needed. - return viewOptions; + // If marker changed, sync it from URL state + if (markerChanged) { + updatedOptions = { + ...updatedOptions, + selectedMarker: urlSelectedMarker, + }; + } + + return updatedOptions; }); } case 'CHANGE_IMPLEMENTATION_FILTER': { diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index 99afc09fce..4930487e1c 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -2246,4 +2246,54 @@ describe('selectedMarker', function () { const queryString = getQueryStringFromState(getState()); expect(queryString).toContain(`marker=${markerIndex}`); }); + + it('syncs selected marker when URL state changes (browser back/forward)', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const threadsKey = getThreadsKey(new Set([0])); + + // Select marker 1 + dispatch(changeSelectedMarker(threadsKey, 1)); + expect( + selectedThreadSelectors.getViewOptions(getState()).selectedMarker + ).toEqual(1); + + // Simulate browser navigation by dispatching UPDATE_URL_STATE + // with a different selected marker (as if we navigated back) + const currentUrlState = urlStateSelectors.getUrlState(getState()); + const newUrlState = { + ...currentUrlState, + profileSpecific: { + ...currentUrlState.profileSpecific, + selectedMarkers: { + [threadsKey]: 2, // Different marker + }, + }, + }; + dispatch({ type: 'UPDATE_URL_STATE', newUrlState }); + + // View state should now sync with the URL state + expect( + selectedThreadSelectors.getViewOptions(getState()).selectedMarker + ).toEqual(2); + + // Now simulate navigating to a state with no selected marker + const urlStateWithNoMarker = { + ...currentUrlState, + profileSpecific: { + ...currentUrlState.profileSpecific, + selectedMarkers: { + [threadsKey]: null, + }, + }, + }; + dispatch({ type: 'UPDATE_URL_STATE', newUrlState: urlStateWithNoMarker }); + + // View state should clear the selected marker + expect( + selectedThreadSelectors.getViewOptions(getState()).selectedMarker + ).toEqual(null); + }); }); From 33174bade6779dbb4e18a6b4b905253d517b5145 Mon Sep 17 00:00:00 2001 From: fatadel Date: Thu, 19 Feb 2026 13:56:05 +0100 Subject: [PATCH 06/11] initialize selected marker from VIEW_FULL_PROFILE instead of separate dispatches --- src/actions/receive-profile.ts | 16 ++++------------ src/reducers/profile-view.ts | 14 ++++++++++++++ src/types/actions.ts | 2 ++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/actions/receive-profile.ts b/src/actions/receive-profile.ts index 7ed898cf6e..25e06a25fb 100644 --- a/src/actions/receive-profile.ts +++ b/src/actions/receive-profile.ts @@ -65,7 +65,7 @@ import { computeDefaultHiddenTracks, getVisibleThreads, } from 'firefox-profiler/profile-logic/tracks'; -import { setDataSource, changeSelectedMarker } from './profile-view'; +import { setDataSource } from './profile-view'; import { fatalError } from './errors'; import { batchLoadDataUrlIcons } from './icons'; import { GOOGLE_STORAGE_BUCKET } from 'firefox-profiler/app-logic/constants'; @@ -359,6 +359,8 @@ export function finalizeFullProfileView( } } + const selectedMarkers = hasUrlInfo ? getSelectedMarkers(getState()) : {}; + withHistoryReplaceStateSync(() => { dispatch({ type: 'VIEW_FULL_PROFILE', @@ -369,20 +371,10 @@ export function finalizeFullProfileView( localTracksByPid, localTrackOrderByPid, timelineType, + selectedMarkers, ...hiddenTracks, }); }); - - // Initialize selected markers from URL state if present - if (hasUrlInfo) { - const selectedMarkers = getSelectedMarkers(getState()); - // Dispatch marker selection for each thread that has a marker in URL - for (const [threadsKey, markerIndex] of Object.entries(selectedMarkers)) { - if (markerIndex !== null) { - dispatch(changeSelectedMarker(threadsKey, markerIndex)); - } - } - } }; } diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index df8d1ae0c7..df1f8e8443 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -174,6 +174,20 @@ const viewOptionsPerThread: Reducer = ( case 'PROFILE_LOADED': // The view options are lazily initialized. Reset to the default values. return {}; + case 'VIEW_FULL_PROFILE': { + // Initialize selectedMarker for each thread from the URL state. + const { selectedMarkers } = action; + const newState: ThreadViewOptionsPerThreads = {}; + for (const [threadsKey, markerIndex] of Object.entries(selectedMarkers)) { + if (markerIndex !== null) { + newState[threadsKey] = { + ..._getThreadViewOptions(state, threadsKey), + selectedMarker: markerIndex, + }; + } + } + return Object.keys(newState).length > 0 ? newState : state; + } case 'BULK_SYMBOLICATION': { const { oldFuncToNewFuncsMaps } = action; // For each thread, apply oldFuncToNewFuncsMap to that thread's diff --git a/src/types/actions.ts b/src/types/actions.ts index 6bcbb12093..c6d18d7dfb 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -39,6 +39,7 @@ import type { ApiQueryError, TableViewOptions, DecodedInstruction, + SelectedMarkersPerThread, } from './state'; import type { CssPixels, StartEndRange, Milliseconds } from './units'; import type { BrowserConnectionStatus } from '../app-logic/browser-connection'; @@ -398,6 +399,7 @@ type ReceiveProfileAction = readonly localTrackOrderByPid: Map; readonly timelineType: TimelineType | null; readonly selectedTab: TabSlug; + readonly selectedMarkers: SelectedMarkersPerThread; } | { readonly type: 'DATA_RELOAD'; From 3864ace732840d34cb836525bf3d27929d483c5f Mon Sep 17 00:00:00 2001 From: fatadel Date: Thu, 19 Feb 2026 16:25:46 +0100 Subject: [PATCH 07/11] remove selectedMarker from view state, instead keep it in url state only --- src/actions/receive-profile.ts | 4 -- src/reducers/profile-view.ts | 68 +++---------------- src/selectors/per-thread/markers.ts | 2 +- src/selectors/url-state.ts | 10 ++- .../__snapshots__/profile-view.test.ts.snap | 1 - src/test/store/profile-view.test.ts | 4 +- src/test/url-handling.test.ts | 46 +++++++------ src/types/actions.ts | 2 - src/types/state.ts | 1 - 9 files changed, 47 insertions(+), 91 deletions(-) diff --git a/src/actions/receive-profile.ts b/src/actions/receive-profile.ts index 25e06a25fb..76909edbdb 100644 --- a/src/actions/receive-profile.ts +++ b/src/actions/receive-profile.ts @@ -41,7 +41,6 @@ import { import { getSelectedTab, getTabFilter, - getSelectedMarkers, } from 'firefox-profiler/selectors/url-state'; import { getTabToThreadIndexesMap, @@ -359,8 +358,6 @@ export function finalizeFullProfileView( } } - const selectedMarkers = hasUrlInfo ? getSelectedMarkers(getState()) : {}; - withHistoryReplaceStateSync(() => { dispatch({ type: 'VIEW_FULL_PROFILE', @@ -371,7 +368,6 @@ export function finalizeFullProfileView( localTracksByPid, localTrackOrderByPid, timelineType, - selectedMarkers, ...hiddenTracks, }); }); diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index df1f8e8443..1a641310e6 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -137,7 +137,6 @@ export const defaultThreadViewOptions: ThreadViewOptions = { selectedInvertedCallNodePath: [], expandedNonInvertedCallNodePaths: new PathSet(), expandedInvertedCallNodePaths: new PathSet(), - selectedMarker: null, selectedNetworkMarker: null, lastSeenTransformCount: 0, }; @@ -174,20 +173,6 @@ const viewOptionsPerThread: Reducer = ( case 'PROFILE_LOADED': // The view options are lazily initialized. Reset to the default values. return {}; - case 'VIEW_FULL_PROFILE': { - // Initialize selectedMarker for each thread from the URL state. - const { selectedMarkers } = action; - const newState: ThreadViewOptionsPerThreads = {}; - for (const [threadsKey, markerIndex] of Object.entries(selectedMarkers)) { - if (markerIndex !== null) { - newState[threadsKey] = { - ..._getThreadViewOptions(state, threadsKey), - selectedMarker: markerIndex, - }; - } - } - return Object.keys(newState).length > 0 ? newState : state; - } case 'BULK_SYMBOLICATION': { const { oldFuncToNewFuncsMaps } = action; // For each thread, apply oldFuncToNewFuncsMap to that thread's @@ -342,10 +327,6 @@ const viewOptionsPerThread: Reducer = ( : { expandedNonInvertedCallNodePaths: expandedCallNodePaths } ); } - case 'CHANGE_SELECTED_MARKER': { - const { threadsKey, selectedMarker } = action; - return _updateThreadViewOptions(state, threadsKey, { selectedMarker }); - } case 'CHANGE_SELECTED_NETWORK_MARKER': { const { threadsKey, selectedNetworkMarker } = action; return _updateThreadViewOptions(state, threadsKey, { @@ -417,44 +398,24 @@ const viewOptionsPerThread: Reducer = ( }); } case 'UPDATE_URL_STATE': { - // When the URL state changes (e.g., via browser back button): - // 1. Check if the transform stack has been popped for each thread. - // If so, reset the stored paths, because they may reference call nodes - // that only exist in a transformed tree. - // See: https://github.com/firefox-devtools/profiler/issues/5689. - // 2. Sync selected marker with URL state. - + // When the URL state changes (e.g., via browser back button), check if the + // transform stack has been popped for each thread. If so, reset the stored paths + // because they may reference call nodes that only exist in a transformed tree. + // See: https://github.com/firefox-devtools/profiler/issues/5689 if (!action.newUrlState) { return state; } - const { transforms, selectedMarkers } = - action.newUrlState.profileSpecific; + const { transforms } = action.newUrlState.profileSpecific; return objectMap(state, (viewOptions, threadsKey) => { const transformStack = transforms[threadsKey] || []; const newTransformCount = transformStack.length; const oldTransformCount = viewOptions.lastSeenTransformCount; - // Get the selected marker from URL state for this thread - const urlSelectedMarker = selectedMarkers[threadsKey] ?? null; - const currentSelectedMarker = viewOptions.selectedMarker; - - // Check if we need to update anything - const transformCountChanged = newTransformCount < oldTransformCount; - const markerChanged = urlSelectedMarker !== currentSelectedMarker; - - if (!transformCountChanged && !markerChanged) { - // No change needed - return viewOptions; - } - - // Build the updated view options - let updatedOptions = { ...viewOptions }; - - // If transform count changed, reset the paths - if (transformCountChanged) { - updatedOptions = { - ...updatedOptions, + // If transform count changed, reset the paths. + if (newTransformCount < oldTransformCount) { + return { + ...viewOptions, selectedNonInvertedCallNodePath: [], selectedInvertedCallNodePath: [], expandedNonInvertedCallNodePaths: new PathSet(), @@ -463,15 +424,8 @@ const viewOptionsPerThread: Reducer = ( }; } - // If marker changed, sync it from URL state - if (markerChanged) { - updatedOptions = { - ...updatedOptions, - selectedMarker: urlSelectedMarker, - }; - } - - return updatedOptions; + // No change needed. + return viewOptions; }); } case 'CHANGE_IMPLEMENTATION_FILTER': { diff --git a/src/selectors/per-thread/markers.ts b/src/selectors/per-thread/markers.ts index 333427b3e7..542eb71afd 100644 --- a/src/selectors/per-thread/markers.ts +++ b/src/selectors/per-thread/markers.ts @@ -552,7 +552,7 @@ export function getMarkerSelectorsPerThread( * This returns the marker index for the currently selected marker. */ const getSelectedMarkerIndex: Selector = (state) => - threadSelectors.getViewOptions(state).selectedMarker; + UrlState.getSelectedMarker(state, threadsKey); /** * From the previous value, this returns the full marker object for the diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 3111c51021..c3ff9e27e6 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -31,7 +31,7 @@ import type { NativeSymbolInfo, TabID, IndexIntoSourceTable, - SelectedMarkersPerThread, + MarkerIndex, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -168,8 +168,6 @@ export const getTabFilter: Selector = (state) => getProfileSpecificState(state).tabFilter; export const hasTabFilter: Selector = (state) => getTabFilter(state) !== null; -export const getSelectedMarkers: Selector = (state) => - getProfileSpecificState(state).selectedMarkers; /** * This selector does a simple lookup in the set of hidden tracks for a PID, and ensures @@ -239,6 +237,12 @@ export const getTransformStack: DangerousSelectorWithArguments< ); }; +export const getSelectedMarker: DangerousSelectorWithArguments< + MarkerIndex | null, + ThreadsKey +> = (state, threadsKey) => + getProfileSpecificState(state).selectedMarkers[threadsKey] ?? null; + export const getIsBottomBoxOpen: Selector = (state) => { const tab = getSelectedTab(state); return getProfileSpecificState(state).isBottomBoxOpenPerPanel[tab]; diff --git a/src/test/store/__snapshots__/profile-view.test.ts.snap b/src/test/store/__snapshots__/profile-view.test.ts.snap index 9a12a5e6a9..c8e4181a52 100644 --- a/src/test/store/__snapshots__/profile-view.test.ts.snap +++ b/src/test/store/__snapshots__/profile-view.test.ts.snap @@ -4428,7 +4428,6 @@ Object { }, "lastSeenTransformCount": 1, "selectedInvertedCallNodePath": Array [], - "selectedMarker": 1, "selectedNetworkMarker": null, "selectedNonInvertedCallNodePath": Array [ 0, diff --git a/src/test/store/profile-view.test.ts b/src/test/store/profile-view.test.ts index 46d1fee453..46c5a627b0 100644 --- a/src/test/store/profile-view.test.ts +++ b/src/test/store/profile-view.test.ts @@ -1046,11 +1046,11 @@ describe('actions/ProfileView', function () { const { dispatch, getState } = storeWithProfile(profile); expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker + selectedThreadSelectors.getSelectedMarkerIndex(getState()) ).toEqual(null); dispatch(ProfileView.changeSelectedMarker(0, 0)); expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker + selectedThreadSelectors.getSelectedMarkerIndex(getState()) ).toEqual(0); }); }); diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index 4930487e1c..7b03d6a78e 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -2036,9 +2036,9 @@ describe('selectedMarker', function () { profile ); - expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker - ).toEqual(markerIndex); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + markerIndex + ); }); it('handles marker index 0 correctly on marker-chart tab', () => { @@ -2121,6 +2121,11 @@ describe('selectedMarker', function () { }); it('survives a URL round-trip on marker-chart tab', () => { + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); const { dispatch, getState } = setup( '?thread=0', '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' @@ -2137,13 +2142,16 @@ describe('selectedMarker', function () { // URL should contain the marker expect(url).toContain(`marker=${markerIndex}`); - // Parse the URL back - const newUrlState = stateFromLocation( - new URL(url, 'https://profiler.firefox.com') + // Load a fresh store from the URL and verify via selector + const { getState: getState2 } = _getStoreWithURL( + { + pathname: + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/', + search: new URL(url, 'https://profiler.firefox.com').search, + }, + profile ); - - // The marker should be in the parsed URL state - expect(newUrlState.profileSpecific.selectedMarkers[threadsKey]).toEqual( + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState2())).toEqual( markerIndex ); }); @@ -2256,9 +2264,9 @@ describe('selectedMarker', function () { // Select marker 1 dispatch(changeSelectedMarker(threadsKey, 1)); - expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker - ).toEqual(1); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + 1 + ); // Simulate browser navigation by dispatching UPDATE_URL_STATE // with a different selected marker (as if we navigated back) @@ -2274,10 +2282,9 @@ describe('selectedMarker', function () { }; dispatch({ type: 'UPDATE_URL_STATE', newUrlState }); - // View state should now sync with the URL state - expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker - ).toEqual(2); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + 2 + ); // Now simulate navigating to a state with no selected marker const urlStateWithNoMarker = { @@ -2291,9 +2298,8 @@ describe('selectedMarker', function () { }; dispatch({ type: 'UPDATE_URL_STATE', newUrlState: urlStateWithNoMarker }); - // View state should clear the selected marker - expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker - ).toEqual(null); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + null + ); }); }); diff --git a/src/types/actions.ts b/src/types/actions.ts index c6d18d7dfb..6bcbb12093 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -39,7 +39,6 @@ import type { ApiQueryError, TableViewOptions, DecodedInstruction, - SelectedMarkersPerThread, } from './state'; import type { CssPixels, StartEndRange, Milliseconds } from './units'; import type { BrowserConnectionStatus } from '../app-logic/browser-connection'; @@ -399,7 +398,6 @@ type ReceiveProfileAction = readonly localTrackOrderByPid: Map; readonly timelineType: TimelineType | null; readonly selectedTab: TabSlug; - readonly selectedMarkers: SelectedMarkersPerThread; } | { readonly type: 'DATA_RELOAD'; diff --git a/src/types/state.ts b/src/types/state.ts index 7cf15967bc..6e18bb2630 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -56,7 +56,6 @@ export type ThreadViewOptions = { readonly selectedInvertedCallNodePath: CallNodePath; readonly expandedNonInvertedCallNodePaths: PathSet; readonly expandedInvertedCallNodePaths: PathSet; - readonly selectedMarker: MarkerIndex | null; readonly selectedNetworkMarker: MarkerIndex | null; // Track the number of transforms to detect when they change via browser // navigation. This helps us know when to reset paths that may be invalid From 008fd2eee22781fa87beb54db36bc151c64fc34a Mon Sep 17 00:00:00 2001 From: fatadel Date: Fri, 20 Feb 2026 12:54:59 +0100 Subject: [PATCH 08/11] replace requestAnimationFrame with containerWidth check for tooltip sync on load --- src/components/shared/chart/Canvas.tsx | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/components/shared/chart/Canvas.tsx b/src/components/shared/chart/Canvas.tsx index 24c3334fc7..ccbcfd0179 100644 --- a/src/components/shared/chart/Canvas.tsx +++ b/src/components/shared/chart/Canvas.tsx @@ -393,12 +393,14 @@ export class ChartCanvas extends React.Component< }; override componentDidMount() { - // Initialize selectedItem from props on mount if provided - // Use requestAnimationFrame to ensure the canvas is fully laid out - if (this.props.selectedItem !== undefined) { - window.requestAnimationFrame(() => { - this._syncSelectedItemFromProp(this.props.selectedItem!); - }); + // Initialize selectedItem from props on mount if provided and the canvas + // already has dimensions. If containerWidth is still 0, the viewport hasn't + // been laid out yet - componentDidUpdate will pick it up once it becomes non-zero. + if ( + this.props.selectedItem !== undefined && + this.props.containerWidth !== 0 + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); } } @@ -430,6 +432,17 @@ export class ChartCanvas extends React.Component< this._syncSelectedItemFromProp(this.props.selectedItem); } + // The canvas just received its dimensions for the first time (containerWidth + // went from 0 to non-zero). If a selectedItem was provided but couldn't be + // synced in componentDidMount, do it now. + if ( + prevProps.containerWidth === 0 && + this.props.containerWidth !== 0 && + this.props.selectedItem !== undefined + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + if ( this.state.selectedItem !== null && prevState.selectedItem === this.state.selectedItem From a6f157a7214b3f8598c2d310365f57ded828c19e Mon Sep 17 00:00:00 2001 From: fatadel Date: Fri, 20 Feb 2026 12:57:26 +0100 Subject: [PATCH 09/11] use SelectedMarkersPerThread type directly --- src/app-logic/url-handling.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 1ec5761733..f27c1e595e 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -41,6 +41,7 @@ import type { Transform, IndexIntoFrameTable, MarkerIndex, + SelectedMarkersPerThread, } from 'firefox-profiler/types'; import { decodeUintArrayFromUrlComponent, @@ -508,7 +509,7 @@ export function stateFromLocation( } // Parse the selected marker for the current thread - const selectedMarkers: { [key: string]: MarkerIndex | null } = {}; + const selectedMarkers: SelectedMarkersPerThread = {}; if ( selectedThreadsKey !== null && query.marker !== undefined && From 680c730f5a68634149baef8fd3cf05516f5ed930 Mon Sep 17 00:00:00 2001 From: fatadel Date: Mon, 23 Feb 2026 11:33:19 +0100 Subject: [PATCH 10/11] scroll to selected marker, if not visible on load --- src/components/marker-chart/Canvas.tsx | 40 +++++++++++++++++++++++++- src/components/shared/chart/Canvas.tsx | 25 ++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/components/marker-chart/Canvas.tsx b/src/components/marker-chart/Canvas.tsx index 6bf0dfb798..0a749cf39c 100644 --- a/src/components/marker-chart/Canvas.tsx +++ b/src/components/marker-chart/Canvas.tsx @@ -147,6 +147,42 @@ function getDefaultMarkerColors(isHighlighted: boolean) { class MarkerChartCanvasImpl extends React.PureComponent { _textMeasurement: TextMeasurement | null = null; + override componentDidUpdate(prevProps: Props) { + // When the viewport finishes sizing itself, or when the selected marker changes, + // scroll to bring the selected marker into view (e.g. on initial load from URL). + const viewportDidMount = + !prevProps.viewport.isSizeSet && this.props.viewport.isSizeSet; + const selectedMarkerChanged = + this.props.selectedMarkerIndex !== prevProps.selectedMarkerIndex; + + if (viewportDidMount || selectedMarkerChanged) { + this._scrollSelectionIntoView(); + } + } + + _scrollSelectionIntoView = () => { + const { selectedMarkerIndex, markerTimingAndBuckets, rowHeight, viewport } = + this.props; + + if (selectedMarkerIndex === null) { + return; + } + + const markerIndexToTimingRow = this._getMarkerIndexToTimingRow( + markerTimingAndBuckets + ); + const rowIndex = markerIndexToTimingRow[selectedMarkerIndex]; + const y: CssPixels = rowIndex * rowHeight; + const { viewportTop, viewportBottom } = viewport; + + if (y < viewportTop || y + rowHeight > viewportBottom) { + // Scroll the marker to the vertical center of the viewport. + const viewportHeight = viewportBottom - viewportTop; + const targetViewportTop = y + rowHeight / 2 - viewportHeight / 2; + viewport.moveViewport(0, viewportTop - targetViewportTop); + } + }; + /** * Get the fill, stroke, and text colors for a marker based on its schema and data. * If the marker schema has a colorField, use that field's value. @@ -1053,7 +1089,8 @@ class MarkerChartCanvasImpl extends React.PureComponent { }; override render() { - const { containerWidth, containerHeight, isDragging } = this.props.viewport; + const { containerWidth, containerHeight, isDragging, viewportTop } = + this.props.viewport; const { selectedMarkerIndex } = this.props; return ( @@ -1074,6 +1111,7 @@ class MarkerChartCanvasImpl extends React.PureComponent { stickyTooltips={true} selectedItem={selectedMarkerIndex} getTooltipPosition={this.getTooltipPosition} + viewportTop={viewportTop} /> ); } diff --git a/src/components/shared/chart/Canvas.tsx b/src/components/shared/chart/Canvas.tsx index ccbcfd0179..dcdd6f395f 100644 --- a/src/components/shared/chart/Canvas.tsx +++ b/src/components/shared/chart/Canvas.tsx @@ -39,6 +39,8 @@ type Props = { readonly getTooltipPosition?: ( item: Item ) => { offsetX: CssPixels; offsetY: CssPixels } | null; + // Current vertical scroll offset of the viewport. + readonly viewportTop?: CssPixels; }; // The naming of the X and Y coordinates here correspond to the ones @@ -382,9 +384,18 @@ export class ChartCanvas extends React.Component< return; } + const { offsetX, offsetY } = tooltipPosition; + + // If the item is outside the visible canvas area, skip setting the tooltip + // position for now. The viewport will scroll to bring it into view, and the + // viewportTop change will trigger a re-sync with the correct position. + if (offsetY < 0 || offsetY > this.props.containerHeight) { + return; + } + const canvasRect = this._canvas.getBoundingClientRect(); - const pageX = canvasRect.left + window.scrollX + tooltipPosition.offsetX; - const pageY = canvasRect.top + window.scrollY + tooltipPosition.offsetY; + const pageX = canvasRect.left + window.scrollX + offsetX; + const pageY = canvasRect.top + window.scrollY + offsetY; this.setState({ selectedItem, pageX, @@ -443,6 +454,16 @@ export class ChartCanvas extends React.Component< this._syncSelectedItemFromProp(this.props.selectedItem); } + // The viewport scrolled (e.g. to bring the selected item into view on + // load). Re-sync the tooltip position so it stays on the selected item. + if ( + this.props.viewportTop !== undefined && + this.props.viewportTop !== prevProps.viewportTop && + this.props.selectedItem !== undefined + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + if ( this.state.selectedItem !== null && prevState.selectedItem === this.state.selectedItem From 2990331229059162d4f64e2ab2f2ed6454f302f0 Mon Sep 17 00:00:00 2001 From: fatadel Date: Mon, 23 Feb 2026 15:24:16 +0100 Subject: [PATCH 11/11] refine comments --- src/components/marker-chart/Canvas.tsx | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/components/marker-chart/Canvas.tsx b/src/components/marker-chart/Canvas.tsx index 0a749cf39c..946002404e 100644 --- a/src/components/marker-chart/Canvas.tsx +++ b/src/components/marker-chart/Canvas.tsx @@ -1015,19 +1015,12 @@ class MarkerChartCanvasImpl extends React.PureComponent { } = this.props; // Step 1: Find which row this marker is displayed in - // markerIndexToTimingRow is a map: markerIndex -> rowIndex const markerIndexToTimingRow = this._getMarkerIndexToTimingRow( markerTimingAndBuckets ); const rowIndex = markerIndexToTimingRow[markerIndex]; - if (rowIndex === undefined) { - // Marker is not in any visible row (might be filtered out) - return null; - } - // Step 2: Get the timing data for all markers in this row - // markerTiming contains arrays of data for all markers in this row const markerTiming = markerTimingAndBuckets[rowIndex]; if (!markerTiming || typeof markerTiming === 'string') { // Row is empty or is a bucket label (string), not actual marker data @@ -1035,9 +1028,6 @@ class MarkerChartCanvasImpl extends React.PureComponent { } // Step 3: Find the position of our specific marker within this row's data - // markerTiming.index[] contains the original marker indexes for all markers in this row - // We need to find which position (markerTimingIndex) corresponds to our markerIndex - // so we can look up its start/end times in the parallel arrays let markerTimingIndex = -1; for (let i = 0; i < markerTiming.length; i++) { if (markerTiming.index[i] === markerIndex) { @@ -1052,7 +1042,6 @@ class MarkerChartCanvasImpl extends React.PureComponent { } // Step 4: Calculate horizontal (X) position - // Get the marker's start and end timestamps from the parallel arrays const startTimestamp = markerTiming.start[markerTimingIndex]; const endTimestamp = markerTiming.end[markerTimingIndex]; @@ -1073,8 +1062,6 @@ class MarkerChartCanvasImpl extends React.PureComponent { const w: CssPixels = ((endTime - startTime) * markerContainerWidth) / viewportLength; - // For instant markers, use the center. For interval markers, use a position near the start - // For instant markers (start === end), use the center point // For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller) const isInstantMarker = startTimestamp === endTimestamp;