From 62f79ab63dbfd345447a5cf7b88f913d02bde9ea Mon Sep 17 00:00:00 2001 From: "GBra 4.669" <35969802+GBra-4-669@users.noreply.github.com> Date: Sun, 4 Oct 2026 03:15:44 +0200 Subject: [PATCH] Invalidate the hovered sample when the thread changes The hovered sample belongs to the thread that was shown when the mouse last moved. Changing the committed range can leave the tooltip pointing at a sample index that's no longer valid or at the wrong sample. The hover is now cleared when the thread changes. Fixes #5690 --- .../shared/thread/ActivityGraph.tsx | 12 ++++ src/components/shared/thread/SampleGraph.tsx | 12 ++++ src/test/components/SampleGraph.test.tsx | 32 ++++++++++- .../components/ThreadActivityGraph.test.tsx | 55 ++++++++++++++++++- 4 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/components/shared/thread/ActivityGraph.tsx b/src/components/shared/thread/ActivityGraph.tsx index 65f1aed588..9649aa5c40 100644 --- a/src/components/shared/thread/ActivityGraph.tsx +++ b/src/components/shared/thread/ActivityGraph.tsx @@ -63,6 +63,7 @@ type State = { hoveredPixelState: null | HoveredPixelState; mouseX: CssPixels; mouseY: CssPixels; + previousThread: Thread | null; }; class ThreadActivityGraphImpl extends React.PureComponent { @@ -72,8 +73,19 @@ class ThreadActivityGraphImpl extends React.PureComponent { hoveredPixelState: null, mouseX: 0, mouseY: 0, + previousThread: null, }; + static getDerivedStateFromProps(props: Props, state: State) { + if (props.rangeFilteredThread !== state.previousThread) { + return { + previousThread: props.rangeFilteredThread, + hoveredPixelState: null, + }; + } + return null; + } + _onMouseLeave = () => { this.setState({ hoveredPixelState: null }); }; diff --git a/src/components/shared/thread/SampleGraph.tsx b/src/components/shared/thread/SampleGraph.tsx index 202341aa35..74c15d4a0c 100644 --- a/src/components/shared/thread/SampleGraph.tsx +++ b/src/components/shared/thread/SampleGraph.tsx @@ -57,6 +57,7 @@ type State = { hoveredPixelState: null | HoveredPixelState; mouseX: CssPixels; mouseY: CssPixels; + previousThread: Thread | null; }; type CanvasProps = { @@ -252,8 +253,19 @@ export class ThreadSampleGraphImpl extends PureComponent { hoveredPixelState: null, mouseX: 0, mouseY: 0, + previousThread: null, }; + static getDerivedStateFromProps(props: Props, state: State) { + if (props.thread !== state.previousThread) { + return { + previousThread: props.thread, + hoveredPixelState: null, + }; + } + return null; + } + _onClick = (event: React.MouseEvent) => { const hoveredSample = this._getSampleAtMouseEvent(event); this.props.onSampleClick(event, hoveredSample?.sample ?? null); diff --git a/src/test/components/SampleGraph.test.tsx b/src/test/components/SampleGraph.test.tsx index 3a3272871a..77f4560feb 100644 --- a/src/test/components/SampleGraph.test.tsx +++ b/src/test/components/SampleGraph.test.tsx @@ -3,11 +3,12 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { Provider } from 'react-redux'; -import { render } from 'firefox-profiler/test/fixtures/testing-library'; +import { render, act } from 'firefox-profiler/test/fixtures/testing-library'; import { fireEvent } from '@testing-library/react'; import { selectedThreadSelectors } from 'firefox-profiler/selectors/per-thread'; import { ensureExists } from 'firefox-profiler/utils/types'; import { TimelineTrackThread } from 'firefox-profiler/components/timeline/TrackThread'; +import { commitRange } from 'firefox-profiler/actions/profile-view'; import type { DrawOperation } from '../fixtures/mocks/canvas-context'; import { autoMockCanvasContext, @@ -234,6 +235,35 @@ describe('SampleGraph', function () { ).toMatchSnapshot(); }); + it('does not show the tooltip anymore when the committed range changes while a sample is hovered', function () { + const { hoverSampleGraph, dispatch } = setup(); + + hoverSampleGraph(7); + expect(document.querySelector('.tooltip')).toBeTruthy(); + + act(() => { + dispatch(commitRange(0, 2)); + }); + + expect(document.querySelector('.tooltip')).toBeFalsy(); + }); + + it('does not keep showing a tooltip for another sample when the committed range changes', function () { + const { hoverSampleGraph, dispatch, getState } = setup(); + + hoverSampleGraph(3); + expect(document.querySelector('.tooltip')).toBeTruthy(); + + act(() => { + dispatch(commitRange(2, 20)); + }); + + expect( + selectedThreadSelectors.getFilteredThread(getState()).samples.length + ).toBeGreaterThan(3); + expect(document.querySelector('.tooltip')).toBeFalsy(); + }); + it('does not show a tooltip when outside of a sample is hovered', function () { const { hoverSampleGraph, getCallNodePath, sampleGraphCanvas } = setup(); diff --git a/src/test/components/ThreadActivityGraph.test.tsx b/src/test/components/ThreadActivityGraph.test.tsx index c9bd961436..75e518db73 100644 --- a/src/test/components/ThreadActivityGraph.test.tsx +++ b/src/test/components/ThreadActivityGraph.test.tsx @@ -10,6 +10,7 @@ import type { import { Provider } from 'react-redux'; import { render, act } from 'firefox-profiler/test/fixtures/testing-library'; +import { fireEvent } from '@testing-library/react'; import { selectedThreadSelectors } from '../../selectors/per-thread'; import { getTimelineType, getSelectedTab } from '../../selectors/url-state'; import { getLastVisibleThreadTabSlug } from '../../selectors/app'; @@ -28,7 +29,12 @@ import { } from '../fixtures/mocks/canvas-context'; import { mockRaf } from '../fixtures/mocks/request-animation-frame'; import { storeWithProfile } from '../fixtures/stores'; -import { fireFullClick } from '../fixtures/utils'; +import { + fireFullClick, + getMouseEvent, + addRootOverlayElement, + removeRootOverlayElement, +} from '../fixtures/utils'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import { autoMockElementSize, @@ -66,6 +72,8 @@ describe('ThreadActivityGraph', function () { autoMockCanvasContext(); autoMockElementSize({ width: GRAPH_WIDTH, height: GRAPH_HEIGHT }); autoMockIntersectionObserver(); + beforeEach(addRootOverlayElement); + afterEach(removeRootOverlayElement); function setup(profile: Profile = getSamplesProfile()) { const store = storeWithProfile(profile); @@ -109,6 +117,21 @@ describe('ThreadActivityGraph', function () { }); } + function hoverActivityGraph( + index: IndexIntoSamplesTable, + graphHeightPercentage: number + ) { + fireEvent( + activityGraphCanvas, + getMouseEvent('mousemove', { + pageX: getSamplesPixelPosition(index), + pageY: GRAPH_HEIGHT * graphHeightPercentage, + offsetX: getSamplesPixelPosition(index), + offsetY: GRAPH_HEIGHT * graphHeightPercentage, + }) + ); + } + // This function gets the selected call node path as a list of function names. function getCallNodePath() { return selectedThreadSelectors @@ -134,6 +157,7 @@ describe('ThreadActivityGraph', function () { threadIndex, activityGraphCanvas, clickActivityGraph, + hoverActivityGraph, getCallNodePath, getContextDrawCalls, }; @@ -262,6 +286,35 @@ describe('ThreadActivityGraph', function () { expect(flushDrawLog()).toMatchSnapshot(); }); + it('does not show the tooltip anymore when the committed range changes while a sample is hovered', function () { + const { dispatch, hoverActivityGraph } = setup(); + + hoverActivityGraph(6, 0.2); + expect(document.querySelector('.tooltip')).toBeTruthy(); + + act(() => { + dispatch(commitRange(0, 5)); + }); + + expect(document.querySelector('.tooltip')).toBeFalsy(); + }); + + it('does not keep showing a tooltip for another sample when the committed range changes', function () { + const { dispatch, getState, hoverActivityGraph } = setup(); + + hoverActivityGraph(3, 0.2); + expect(document.querySelector('.tooltip')).toBeTruthy(); + + act(() => { + dispatch(commitRange(2, 20)); + }); + + expect( + selectedThreadSelectors.getRangeFilteredThread(getState()).samples.length + ).toBeGreaterThan(3); + expect(document.querySelector('.tooltip')).toBeFalsy(); + }); + it('selects the full call node path when clicked', function () { const { clickActivityGraph, getCallNodePath } = setup();