2048382 - Nova updates for tab close button and tab mute button

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, task, P2)

Description

[Stephen Thompson [:sthompson]]

Attachment: nova-tab-mute-close-buttons.mov

After bug 2023619, the tab mute button and tab close buttons do not follow the Nova spec. This is likely because these buttons are only specced out in the Nova Components file https://www.figma.com/design/PqfaOcMGbX5liEXTTUzeYX/Nova-Components--Experimental-?node-id=9819-13980&m=dev

Looks like both of these buttons should be round and have a colored hover state. They are inconsistent right now, mostly since the close button is a custom styled <image> while the mute button is a <moz-button>.

• horizontal and vertical expanded tab strip (20x20 round button)
• pinned/vertical collapsed tab strip (16x16 round button with box shadow and a border on hover)

The vertical collapsed tab strip close button is 15x15 and squashing its 12x12 close icon into an 8x8 content box. It should be more like the vertical collapsed tab strip mute button that's 16x16 and puts the icon into a 12x12 icon box.

Comment 2

[Stephen Thompson [:sthompson]]

Note from Jules in bug 2047691 about the mute button: "We can probably alias off the tab radius."

Comment 3

[kpatenio]

Attachment: Bug 2048382 — update tab mute and close buttons for Nova

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/75f04a72c569
https://hg.mozilla.org/integration/autoland/rev/92313f0da4ac
— update tab mute and close buttons for Nova r=desktop-theme-reviewers,tabbrowser-reviewers,niklas,dao

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f8c395c128c2
https://hg.mozilla.org/integration/autoland/rev/31ad1892c2cd
Revert "Bug 2048382 — update tab mute and close buttons for Nova r=desktop-theme-reviewers,tabbrowser-reviewers,niklas,dao" for causing mochitests failures in browser_delay_autoplay_silentAudioTrack_media.js.

Comment 6

[Serban Stanca [:SerbanS]]

Reverted this because it was causing mochitests failures in browser_delay_autoplay_silentAudioTrack_media.js.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_delay_autoplay_silentAudioTrack_media.js | should_not_show_sound_indicator_after_resume_tab - Test timed out

Comment 7

[kpatenio]

Hm, the test might be failing in https://searchfox.org/firefox-main/rev/2916f047242de012d1aa0c572be9e5239644cc48/toolkit/content/tests/browser/browser_delay_autoplay_silentAudioTrack_media.js#13. It synthesizes mouse events at various set coordinates https://searchfox.org/firefox-main/rev/2916f047242de012d1aa0c572be9e5239644cc48/browser/components/tabbrowser/test/browser/tabs/head.js#215-218, which I suspect doesn't work nicely with the updated overlay button styles.

Comment 8

[kpatenio]

The test fails locally for me. I can get it to pass if I set these css changes in .tab-audio-button:

  --button-size-icon-small: 24px; /* Reverted to Proton size */
  --button-min-height-small: var(--tab-content-button-size); /* Kept from the patch, 20px, for Nova */
  --button-border-radius: var(--border-radius-small); /* Reverted, no longer gate to just Proton */

When we set --button-size-icon-small to 20px (which is the value of --tab-content-button-size in the patch for Nova), then the test hangs, failing because the expected tooltip is never shown. I posted the wrong head.js link in Comment 7. It should be in https://searchfox.org/firefox-main/rev/017a9913bd6c272a859aea16d3cab5a4d46bf1b0/toolkit/content/tests/browser/head.js#153-158.

I first suspected that the smaller button size, combined with the rounded radius, was the cause of the failure. Waiting for a mouseover event though before attempting to mousemove in hover_icon seemed to fix it:

async function hover_icon(icon, tooltip) {
  disable_non_test_mouse(true);

  let mouseOverPromise = BrowserTestUtils.waitForEvent(icon, "mouseover");
  let popupShownPromise = BrowserTestUtils.waitForEvent(tooltip, "popupshown");
  EventUtils.synthesizeMouse(icon, 1, 1, { type: "mouseover" });
  await mouseOverPromise;
  EventUtils.synthesizeMouse(icon, 2, 2, { type: "mousemove" });
  EventUtils.synthesizeMouse(icon, 3, 3, { type: "mousemove" });
  EventUtils.synthesizeMouse(icon, 4, 4, { type: "mousemove" });
  await popupShownPromise;
}

The test passed with this change, even with ./mach mochitest --headless --verify toolkit/content/tests/browser/browser_delay_autoplay_silentAudioTrack_media.js. Might have to double check with a try push.

Comment 9

[kpatenio]

hover_icon in https://searchfox.org/firefox-main/rev/017a9913bd6c272a859aea16d3cab5a4d46bf1b0/toolkit/content/tests/browser/head.js#153-158 has a similar implementation and is used by the test https://searchfox.org/firefox-main/rev/017a9913bd6c272a859aea16d3cab5a4d46bf1b0/browser/components/tabbrowser/test/browser/tabs/browser_audioTabIcon.js#90. I didn't observe any failures running browser/components/tabbrowser/test/browser/tabs/browser_audioTabIcon.js. Making the same changes for this file actually caused the test to fail, so I don't see a need to wait for a mouseover event there.

Comment 10

[kpatenio]

The original test failure seems to have been fixed, but I observed more when I ran try pushes (see my comments in https://phabricator.services.mozilla.com/D314533). Still investigating.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9e896fb277f6
https://hg.mozilla.org/integration/autoland/rev/2114b7085d1e
— update tab mute and close buttons for Nova r=desktop-theme-reviewers,tabbrowser-reviewers,niklas,dao,sthompson

Comment 12

[kpatenio]

Re-landed the patch. I updated several helper functions for tabbrowser/ tests to try reduce flakiness.

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/2114b7085d1e