2065170 - New Tab's unscoped panel-item::part(button) rule strips the icon slot from panel-items it doesn't own

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Dão Gottwald [:dao]]

browser/extensions/newtab/content-src/styles/activity-stream.scss has:

panel-item::part(button) {
  padding-inline-start: var(--space-medium);
}

The selector is unscoped, so it matches every panel-item in the New Tab document. This is wrong for panel-lists with icons, causing the labels to overlap the icons.

Added in bug 2012765 for New Tab's own menus, which carry no icons, so nothing showed the damage at the time. The search mode switcher's panel-list is the first one with icons embedded in New Tab, and its engine names overlap their icons.

Scoping the rule to New Tab's own menus would fix it for any panel-item with icons embedded in the page. Note that browser/themes/shared/urlbar.css currently sets padding-inline-start on the switcher's items unconditionally, which masks this; that override is what bug 2032017 would otherwise have been able to drop under Nova.

Comment 1

[Hackbot]

Confirmed by reading the code, plus a couple of details worth having on the bug.

Root cause. browser/extensions/newtab/content-src/styles/activity-stream.scss sets 12px (--space-medium), overriding the toolkit default of 32px in toolkit/content/widgets/panel-list/panel-item.tokens.css (calc(--panel-menuitem-padding-inline + --icon-size + --space-small) under Nova, line 31). That 32px is exactly what clears the icon: panel-item.css paints the icon as a background-image on the button at --panel-menuitem-padding-inline (8px) at 16px, so a 12px label start lands inside the 8-24px icon box. An outer-document ::part() rule always wins over the shadow tree, so the widget can't defend itself.

Note this has already bitten New Tab's own menus twice, wherever a type="checkbox"[checked] item paints the toolkit checkmark: _Stocks.scss and _Lists.scss both counter-override it back to the toolkit value.

Suggested fix — make the reduced padding opt-in rather than document-wide:

1. Add a shared class (e.g. newtab-panel-list) to the <panel-list> elements New Tab renders — 26 call sites, all in content-src/components/**, plus the shared SizeSubmenu.jsx / MoveSubmenu.jsx submenus.
2. Replace the bare rule with .newtab-panel-list > panel-item { --panel-item-button-padding-inline-start: var(--space-medium); }. Setting the token rather than ::part(button) { padding-inline-start } is worth doing here: it's the sanctioned knob, and a document-tree declaration on the host still beats the shadow :host default, so it behaves the same without burning the ::part override.
3. Don't opt in the panel-lists that contain checkbox items (#stocks-list-menu, #lists-switcher-panel, the SizeSubmenu lists) — then the two counter-overrides above can be deleted rather than left as accumulating patches.

A narrower :not(.searchmode-switcher-panel-list) > panel-item::part(button) would unblock the switcher, but it only postpones this for the next icon-bearing panel-list embedded in the page.

One correction: the masking override is in browser/themes/shared/urlbar-searchbar.css, not urlbar.css. Relevant to bug 2032017: that sheet is only imported by browser-shared.css and smartbar.css, i.e. not into the New Tab document — which is why the switcher's items overlap there while chrome windows look fine, and why dropping the override under Nova is blocked on this bug rather than on the newtab side.

Tests: nothing asserts panel-item padding. Closest behavioural coverage is browser/extensions/newtab/test/browser/browser_newtab_last_LinkMenu.js and browser/components/urlbar/tests/browser/searchbar/browser_searchModeSwitcher.js; a computed-style check on the switcher's panel-item::part(button) in the newtab document would be the cheap regression test.

Severity: suggest S3 — purely visual, no functionality lost, and currently confined to the in-development newtab <moz-urlbar>, but it makes engine names unreadable and it blocks bug 2032017. (Not verified by running; reasoned from the source.)

---

If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-wrong-file, ai-triage-wrong-cause, ai-triage-hallucination, ai-triage-out-of-scope.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 2065170 - Scope New Tab's panel-item padding to its own menus. r?ini!,daleharvey!

Every panel-list New Tab owns takes the class, so its own menus are unchanged. The switcher's override goes with it, leaving panel-item to reserve the icon's room itself.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c0eadb5aab89
https://hg.mozilla.org/integration/autoland/rev/5b026049a5b8
Scope New Tab's panel-item padding to its own menus. r=ini,daleharvey,desktop-theme-reviewers,urlbar-reviewers,kcochrane

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/5b026049a5b8