2054065 - [Nova] Switching tabs and interacting with extensions popup have become laggy in windows with large numbers of tabs

Status: RESOLVED FIXED

(Firefox :: Sidebar, defect, P1)

Description

[Kestrel]

STR:

1. Launch latest Nightly 154.0a1 with browser.nova.enabled = true.
2. Use a session with a few thousand unloaded tabs.
3. Install some extensions.
4. Click extensions toolbar button and hover cursor over items.

Expected:
Popup opens quickly and hovered state responds immediately regardless of number of tabs like before.

Actual:
Popup opens slowly and hovered state takes seconds to respond, proportional to the number of tabs.

Happens regardless of sidebar visibility.

Performance profile:
https://share.firefox.dev/4vVOad5

Caused by sidebar.css addition:

  :root:has(#tabbrowser-tabpanels[splitview] .split-view-panel[column="0"].deck-selected) #sidebar-box:not([sidebar-positionend]) + #sidebar-splitter,
  :root:has(#tabbrowser-tabpanels[splitview] .split-view-panel[column="0"].deck-selected)
    #sidebar-container:not([sidebar-panel-open]):not([sidebar-positionend])
    + #sidebar-launcher-splitter {
    margin-inline-end: calc(-1 * var(--chrome-window-gap) + 3px);
  }

  :root:has(#tabbrowser-tabpanels[splitview] .split-view-panel[column="1"].deck-selected) #sidebar-box[sidebar-positionend] + #sidebar-splitter,
  :root:has(#tabbrowser-tabpanels[splitview] .split-view-panel[column="1"].deck-selected)
    #sidebar-container[sidebar-positionend]:not([sidebar-panel-open])
    + #sidebar-launcher-splitter {
    margin-inline-start: calc(-1 * var(--chrome-window-gap) + 3px);
  }

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b9f53b8fb9dde343729b698e5202cf47284f53ef&tochange=1e2fcbc619b00e3645e6408492ccd9e4ac90ac55

Regressed by Bug 2023685.

Comment 1

[BugBot [:suhaib / :marco]]

:kcochrane, since you are the author of the regressor, bug 2023685, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2023685

Comment 3

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:jsudiaman, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Kestrel]

The same sidebar CSS selector also significantly slows tab switching with large numbers of tabs.

Performance profile when switching between two basic tabs, about:blank and about:mozilla:
Nova disabled: https://share.firefox.dev/3TpNrm0 (200ms jank)
Nova enabled: https://share.firefox.dev/4fCvzvy (600ms jank)

Comment 5

[Sarah Clements [:sclements] | PTO until Sept 8th]

Marcus, Emilio, any thoughts on this? I haven't validated whether these recent changes to sidebar would've caused issues .. not sure why they would.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

:root:has() causes tons of extra work on every DOM mutation and invalidation. Please use something else to detect the state you care about.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Can we add a lint to prevent the use of :has() in the main window? It's going to bite us for sure. Dao, wdyt? I could try to add one if given some direction :)

Comment 8

[Dão Gottwald [:dao]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #7)

Can we add a lint to prevent the use of :has() in the main window? It's going to bite us for sure. Dao, wdyt? I could try to add one if given some direction :)

Agreed, thanks for posting a patch in bug 2060862.

Comment 9

[Sarah Clements [:sclements] | PTO until Sept 8th]

Kelly, could you pick this up next please? You'll need to change the use of :root:has() in browser/themes/shared/tabbrowser/content-area.css and browser/themes/shared/sidebar.css where emilio has added estlint-disable rule in bug 2060862.

Comment 10

[Kelly Cochrane [:kcochrane]]

Attachment: Bug 2054065 - Rework instances of :root:has() in sidebar.css and content-area.css to resolve perf regression

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/96c87b9a6e63
https://hg.mozilla.org/integration/autoland/rev/fea353a5dcf4
Rework instances of :root:has() in sidebar.css and content-area.css to resolve perf regression r=desktop-theme-reviewers,tabbrowser-reviewers,jsudiaman,dao

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/fea353a5dcf4

Comment 13

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected, along with ESR.
:kcochrane, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta and ESR approvals.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox154 and the ESR status flag(s) to wontfix.

For more information, please visit BugBot documentation.