1696621 - Profiler - change animation to be the same as for the other [list_item >](s) added to the Customize Overflow Menu

Status: NEW

(Firefox :: Toolbars and Customization, defect, P5)

Description

[Cristian Fogel [:cfogel]]

Attachment: issueeee.gif

Affected versions

• 88.0a1(2021-05-03); 87.0b5;

Affected platforms

• Windows 10, macOS 10.13

Steps to reproduce

1. Launch Firefox, access the Customize page(right click on toolbar);
2. Add several buttons on the Overflow section;
3. Add the Profiler & Froget buttons as well;
4. Grab the Profiler button / do the same for Forget button;

Expected result

• items that have " >" at the right edge should be consistent across one another;

Actual result

• Profiler item has different animation (expanding across area instead of just the icon being animated) as opposed to Forget/TextEncoding and the other items;

Regression range

• not a regression, initial implementation did not have any animation; marking as bad the build/date when the current implementation was added:
• First bad: 2021-01-16;
• Last good: 2021-01-15;
• Pushlog: URL;
• Potential "regressor": bug 1650835 looks like the candidate;

Additional notes

• attached recording to illustrate the "issue";
• this is not a regression, just that the item might be better suited to have the same animation for consistency.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug is a defect, but please change it back in case of error.

Comment 2

[Julien Wajsberg [:julienw]]

I investigated a bit, and especially I compared with the "History" button, which is similar (it also has a arrow at the right).
The Profiler button is special, because it has a "drop marker" (the small arrow when it's added to the toolbar). This is what makes it behave specially here.

Indeed normal buttons have this DOM hierarchy:

<toolbarpaletteitem>
  <toolbarbutton>
    <image>
    <button>
    ::after

(::after is for the arrow at the right, not all buttons have it)

while the profiler button has this DOM hierarchy:

<toolbarpaletteitem>
  <toolbaritem class="toolbaritem-combined-buttons chromeclass-toolbar-additional">
    <toolbarbutton>
      <image>
      <button>
      ::after

I could see 2 important differences when added in the overflow menu:

1. The difference on mousedown, which is the initial issue in this bug.
   This is how the Profiler button reacts [1]:

toolbarpaletteitem[mousedown] > toolbaritem.toolbaritem-combined-buttons {
  transform: scale(1.1);
}

But this is how the History button reacts [2]:

toolbarpaletteitem[mousedown] > toolbarbutton > .toolbarbutton-icon,
toolbarpaletteitem[mousedown] > toolbarbutton > .toolbarbutton-badge-stack > .toolbarbutton-icon {
  transform: scale(1.3);
}

As you see, the presence of this toolbaritem makes this difference. I would say that the first transform is incorrect (do we have more "combined buttons"?), and we should add the possibility of a toolbaritem.

2. The opacity for the arrow icon.

For the History button, this comes from [3]:

toolbarpaletteitem[place="menu-panel"] > .subviewbutton-nav::after {
  opacity: 0.5;
}

As you see, the fact we have a toolbaritem makes that we miss this rule again.

3. Also do we need so strict CSS selectors with the "direct child" compound operator?
   I know there's this explanation around "efficient CSS selectors" but I believe it's very minor when the right element of the selector is already selective enough, and even in other cases when we use class selectors anywhere, because we have bloom filters for class selectors (IIRC, I'm far from being an expert here). And the price to pay is that it's easier to miss cases.

[1] https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/browser/themes/shared/customizableui/customizeMode.inc.css#213-215
[2] https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/browser/themes/shared/customizableui/customizeMode.inc.css#207-210
[3] https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/browser/themes/shared/customizableui/panelUI.inc.css#1830-1832

From this analysis I'm moving this to the Customization component.

Comment 3

[:Gijs (out for now; he/him)]

Maybe bug 1698436 improved things here?

Comment 4

[Julien Wajsberg [:julienw]]

Currently I don't see a difference, even with proton enabled, should I enable more prefs to try this change?

Comment 5

[:Gijs (out for now; he/him)]

Per the last few comments, maybe this is fixed now?

Comment 6

[Julien Wajsberg [:julienw]]

Ah, I see how my comment 4 could be confusing.

I meant that either with or without proton enabled, I still see the issue as described.