1834312 - The Sort By dropdown arrow in about:logins is overlapped by text, if the sorting criteria is a longer string - (e.g. Username (Z-A))

Status: VERIFIED FIXED

(Firefox :: about:logins, defect)

Description

[Dorin Pop]

Attachment: arrow overlap

[Affected versions]:

• Nightly 115.0a1 (build id: 20230521210743)
• Beta 114.0b7 (build id: 20230521180321)
• Firefox 113.0.1 (build id: 20230511191846)

[Affected Platforms]:

• Mac 12.6.2
• Windows 10 x64
• Ubuntu 20.04 x64

[Prerequisites]:
Have at least 1 saved login

[Steps]:

1. Go to about:logins
2. In the left search column, sort the logins by Username (Z-A)

[Expected]:
The dropdown arrow is displayed next to the Username (Z-A) string

[Actual]:
The dropdown arrow is overlapped by text

[Regression]:
Last good revision: 18a5d13768562d011440560c5a6a5bbde9d589b1
First bad revision: 32be81d9b0cd316d794d6d3bdefef4b019542597
INFO: Pushlog: Link

Comment 1

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

:julienw, since you are the author of the regressor, bug 1816131, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Julien Wajsberg [:julienw]]

I think we need to add this to #login-sort:

background-position: right 3px center;

(and of course have something for RTL too)

What do you think itiel? Do you see a better idea? Should we add some shared style for such compact select boxes?

Comment 3

[Itiel]

(In reply to Julien Wajsberg [:julienw] from comment #2)

I think we need to add this to #login-sort:

background-position: right 3px center;

(and of course have something for RTL too)

What do you think itiel? Do you see a better idea? Should we add some shared style for such compact select boxes?

Oof, this is annoying :(
I like your idea of adding a compact select class, but how compact is "compact"? Future UIs specs may require other spacing that may be somewhere between "normal" and "compact", etc...
I think to make this easier to fix for next time (hopefully there won't be a next time but we said it last time as well...), adding vars for the inline-start and inline-end paddings would allow overriding them without specifying the RTL variants (for the background position you mentioned above).
Of course the background position would need to be calculated according to this, maybe something like calc((var(--select-padding-inline-end) - 12px /* = background-image size /*) / 2) to center the arrow in the end.

I'm not feeling well at the moment so I can't work on it soon-ish, if you're not up for the task bounce the needinfo back to me and I'll work on it when I can :)

Comment 4

[Julien Wajsberg [:julienw]]

Maybe we should add the background image as a generated content instead, so that we don't have to tweak the background-position. I'll look if this is doable.

Comment 5

[Julien Wajsberg [:julienw]]

Attachment: Bug 1834312 - Use CSS variables to control the <select> internal padding r=itiel

Various properties depend on the padding value. Using variables makes
this more explicit, and enable users to change it easily.

Comment 6

[Julien Wajsberg [:julienw]]

Attachment: Bug 1834312 - Fix the style for the Sort-By <select> in about:logins r=itiel!,sgalich!

This also cleans up the style around this select, now that it's been
shared in commons-shared.css

Depends on D179077

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to Julien Wajsberg [:julienw] from comment #4)

Maybe we should add the background image as a generated content instead, so that we don't have to tweak the background-position. I'll look if this is doable.

FTR this doesn't work, I believe because <select> is somewhat special.

Comment 8

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

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/330f7579fef8
Use CSS variables to control the <select> internal padding r=Itiel,desktop-theme-reviewers,sfoster
https://hg.mozilla.org/mozilla-central/rev/1567e1db9b44
Fix the style for the Sort-By <select> in about:logins r=Itiel,credential-management-reviewers,mtigley

Comment 10

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/330f7579fef8
https://hg.mozilla.org/mozilla-central/rev/1567e1db9b44

Comment 11

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Dorin Pop]

I have verified that the issue is no longer reproducible on the latest Nightly
Version 116.0a1
Build ID 20230613152538
MacOS Monterey 12.6.2

Comment 13

[Julien Wajsberg [:julienw]]

This shipped in several releases already, so I don't think there's an urgency in fixing this. As this bug proves, touching shared CSS can easily bring regressions, so I'd rather let this bake in Beta a bit.
Please let me know if you disagree.