Add a 'filter' button next to the tooltip label of markers. by fqueze · Pull Request #5626 · firefox-devtools/profiler

fqueze · 2025-10-03T14:16:28Z

This should be especially useful to quickly update the filter box based on the label of an interesting marker that has been selected.

Relevant example profile: https://share.firefox.dev/42kDTuf

The result after clicking the filter button: https://share.firefox.dev/4gQHPsx

codecov · 2025-10-03T14:21:59Z

Codecov Report

❌ Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.76%. Comparing base (f566a5b) to head (8e92259).

Files with missing lines	Patch %	Lines
src/profile-logic/marker-schema.tsx	91.89%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5626      +/-   ##
==========================================
+ Coverage   85.75%   85.76%   +0.01%     
==========================================
  Files         311      311              
  Lines       30703    30747      +44     
  Branches     8443     8455      +12     
==========================================
+ Hits        26329    26370      +41     
- Misses       3950     3953       +3     
  Partials      424      424

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

canova

Thanks for the PR. Requesting changes for the place where we decide not to show the filter button for labels with a space in them. Please see my comment below.

canova · 2025-10-16T10:15:59Z

-    const { className } = this.props;
+    const { className, markerIndex, getMarkerLabel } = this.props;
+    const markerLabel = getMarkerLabel(markerIndex);
+    const filterButtonTitle = `Only show "${markerLabel}" markers`;


Can we also make this localized?

canova · 2025-10-16T10:23:32Z

            {this._maybeRenderMarkerDuration()}
-            <div className="tooltipTitle">{this._renderTitle()}</div>
+            <div className="tooltipTitle">
+              {markerLabel.includes(' ') ? (


Why do we not print the filter button when there is a space? This looks arbitrary and will probably make our users confused.

Edit: I think you did it this way because the title might actually include other payload fields. But this seems very strange UX-wise because a single marker field/marker name might still include spaces. For example, we have markers that has "CSS Transition" in the name. So because of this check, we won't be able to filter that.

I think we should have a different solution instead. Instead of passing markerLabel to changeMarkersSearchString we can just pass the marker name instead. If you want to be able to still filter by not just marker names, but with other fields too. Then we should have a different approach of figuring out if the markerLabel contains multiple fields. One solution that comes to my mind is to extract all the fields from the markerLabel and then create a comma separated list from these, so we can still search by that specifically.

For example, while I was testing it locally, I had this marker: beforescriptexecute - DOMEvent, and the tooltipLabel schema field for that marker is "{marker.data.eventType} - DOMEvent". Instead we can just extact the marker.data.eventType from there and search for it. If a marker lable contains this "{marker.data.eventType} - {marker.data.name}, we can extact both and create a search string with "{marker.data.eventType},{marker.data.name} instead that can be searched properly with the logical AND.

This can be done in a selector level instead, for example we have getMarkerTooltipLabelGetter for the label getter. We can have another getter for the tooltip label search term.

canova · 2025-10-16T10:25:12Z

+    if (
+      prevProps.defaultValue !== this.props.defaultValue &&
+      this.props.defaultValue !== null
+    ) {
+      this._previouslyNotifiedValue = this.props.defaultValue;
+    }
+  }


Hmm, checking for defaultValue looks counter-intuitive here.

canova · 2025-10-16T11:01:32Z

+    // think the value hasn't changed.
+    if (
+      prevProps.defaultValue !== this.props.defaultValue &&
+      this.props.defaultValue !== null


Also, why do we check for this.props.defaultValue !== null here. What happens if this value gets updated from "something" to null.

canova · 2025-10-21T11:20:36Z

+    .title = Only show "{ $markerName }" markers
+    .aria-label = Only show "{ $markerName }" markers


Actually this is not really accurate. We are not showing only "$markerName markers. We are filtering every marker by that name.
I think it should be something like:

Only show markers matching: “{ $markerName }”

We have something similar here:

profiler/locales/en-US/app.ftl

Line 1139 in f566a5b

TransformNavigator--drop-samples-outside-of-markers-matching = Drop samples outside of markers matching: “{ $item }”

canova

Looks good to me with some nits, thanks!

canova · 2025-10-21T11:39:33Z

+  markerSchema: MarkerSchema,
+  stringTable: StringTable,
+  label: string
+): string {


let's return string | null here and return null in cases we return ''.

canova · 2025-10-21T11:41:23Z

+  if (keys.length !== 3) {
+    return '';
+  }


I guess people can write marker.name at the start of the tooltip. But I guess that will be handled in the getSearchTermGetter if it's not found.

canova · 2025-10-21T11:59:40Z

+      // Note: The `.` in the pattern matches bidi isolation characters that Fluent
+      // adds around variables for proper text directionality
+      const filterButton = screen.getByRole('button', {
+        name: /only show markers matching: “.Reflow.”/i,


We usually write the isolation characters specifically, for example:

profiler/src/test/components/Timeline.test.tsx

Line 1258 in f566a5b

'Only show “\u2068Parent Process\u2069”'

canova · 2025-10-21T12:06:19Z

+  flex: none;
+  padding: 0;
+  border: none;
+  margin-left: 6px;


let's use margin-inline-start

canova · 2025-10-21T12:10:16Z

+  background-image: url(../../../res/img/svg/filter.svg);
+  background-position: center;
+  background-repeat: no-repeat;
+  background-size: 16px 16px;


I don't think this is necessary.

canova

LGTM, thanks!

Changes: [Markus Stange] Streamline some code related to profile publishing (#5608) [Nazım Can Altınova] Implement fetching JS sources from browser via WebChannel (#5506) [Nisarg Jhaveri] Remove "Back to home" link on import errors when not loading from file (#5279) [Florian Quèze] Show the 'inl' badge on inlined frames showing in marker stacks. (#5628) [Florian Quèze] Add a 'filter' button next to the tooltip label of markers. (#5626) And thanks to our localizers: de: Michael Köhler el: Jim Spentzos en-GB: Ian Neal es-CL: ravmn fr: brumedautomne127 fur: Fabio Tomat fy-NL: Fjoerfoks ia: Melo46 it: Francesco Lodolo [:flod] nl: Mark Heijl pt-BR: Marcelo Ghelman ru: Valery Ledovskoy sv-SE: Andreas Pettersson tr: Grk, Selim Şumlu zh-CN: Olvcpr423 zh-TW: Pin-guang Chen

fqueze requested a review from mstange October 3, 2025 14:16

canova requested review from canova and removed request for mstange October 14, 2025 08:59

canova requested changes Oct 16, 2025

View reviewed changes

canova reviewed Oct 16, 2025

View reviewed changes

fqueze force-pushed the filter-markers-from-tooltip branch from 0c8cd2c to 35d8666 Compare October 16, 2025 16:50

fqueze requested a review from a team as a code owner October 16, 2025 16:50

flodolo approved these changes Oct 16, 2025

View reviewed changes

canova reviewed Oct 21, 2025

View reviewed changes

fqueze force-pushed the filter-markers-from-tooltip branch from 35d8666 to 8e92259 Compare October 21, 2025 11:51

canova reviewed Oct 21, 2025

View reviewed changes

Add a 'filter' button next to the tooltip label of markers.

68194b6

fqueze force-pushed the filter-markers-from-tooltip branch from 8e92259 to 68194b6 Compare October 21, 2025 12:48

canova approved these changes Oct 21, 2025

View reviewed changes

Merge branch 'main' into filter-markers-from-tooltip

b7b84d6

canova enabled auto-merge (squash) October 21, 2025 12:53

canova merged commit 9d65777 into firefox-devtools:main Oct 21, 2025
12 of 13 checks passed

canova mentioned this pull request Oct 21, 2025

Deploy October 21, 2025 #5638

Merged

		.title = Only show "{ $markerName }" markers
		.aria-label = Only show "{ $markerName }" markers

Conversation

fqueze commented Oct 3, 2025

Uh oh!

codecov Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Oct 3, 2025 •

edited

Loading