1673233 - With 4 tabs and long tab names on about:certificate, the first tab text can overflow off-view

Status: RESOLVED FIXED

(Firefox :: Security, defect, P3)

Description

[Itiel]

Attachment: Screenshot

STR:

1. Go to https://english.president.gov.tw/
2. Open the site's certification
3. Click on the second, third or fourth tab

AR:
The first tab text gets overflowed off-view

ER:
Text in all tabs should be visible

Comment 1

[:Gijs (he/him)]

Although I understand why this was marked as a regression, this can presumably happen to any certificate with a sufficient number of items in the chain, where at least one of the labels cannot be wrapped or elided, which isn't something that was introduced in bug 1659293.

I also actually can't reproduce on en-US nightly. The "(unknown)" string gets elided into "(unkn..." , which makes sense because the tab label has overflow: hidden and text-overflow: ellipsis. Is this effect locale-dependent?

(All that said, I am surprised it says "(unknown)"... which is a separate issue with that patch, I think...)

Comment 2

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #1)

(All that said, I am surprised it says "(unknown)"... which is a separate issue with that patch, I think...)

Put up a patch for this in bug 1673325.

Comment 3

[Itiel]

(In reply to :Gijs (he/him) from comment #1)

I also actually can't reproduce on en-US nightly. The "(unknown)" string gets elided into "(unkn..." , which makes sense because the tab label has overflow: hidden and text-overflow: ellipsis. Is this effect locale-dependent?

Sigh. This somewhat seems to be my fault, as I have added unicode-bidi: plaintext in bug 1671373 to make the tabs text auto directional, but it seems it doesn't play nicely if the element inherits direction: rtl :(

This is my plan for a fix, let me know if this sounds good for you:

1. Revert the logic for dir-auto added in bug 1671373
2. Force LTR for the tabs instead of the line here: https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/toolkit/components/certviewer/content/components/certificate-tabs-section.js#56
   2a. How would I target the "unknown" tab so it'd not get LTR'd?
3. To not regress 1671373 wrt the border-radius, target the element according to its parent direction and set the radius with no logical properties

Comment 4

[:Gijs (he/him)]

(In reply to Itiel from comment #3)

(In reply to :Gijs (he/him) from comment #1)

I also actually can't reproduce on en-US nightly. The "(unknown)" string gets elided into "(unkn..." , which makes sense because the tab label has overflow: hidden and text-overflow: ellipsis. Is this effect locale-dependent?

Sigh. This somewhat seems to be my fault, as I have added unicode-bidi: plaintext in bug 1671373 to make the tabs text auto directional, but it seems it doesn't play nicely if the element inherits direction: rtl :(

This is my plan for a fix, let me know if this sounds good for you:

1. Revert the logic for dir-auto added in bug 1671373
2. Force LTR for the tabs instead of the line here: https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/toolkit/components/certviewer/content/components/certificate-tabs-section.js#56
   2a. How would I target the "unknown" tab so it'd not get LTR'd?

You can probably use an attribute selector for [data-l10n-id] to distinguish that case ?

3. To not regress 1671373 wrt the border-radius, target the element according to its parent direction and set the radius with no logical properties

Hm, can we not just set the radius on the parent?

But yeah, this sounds reasonable. Thank you!

Comment 5

[Itiel]

(In reply to :Gijs (he/him) from comment #4)

2a. How would I target the "unknown" tab so it'd not get LTR'd?

You can probably use an attribute selector for [data-l10n-id] to distinguish that case ?

I thought about that, but the ID may change over time for various reasons...
I think I'll just set dir=ltr in here: https://searchfox.org/mozilla-central/rev/e1d1f043957191616721b9e8bf811c0aab8a203a/toolkit/components/certviewer/content/components/certificate-tabs-section.js#51 if !tabName.

3. To not regress 1671373 wrt the border-radius, target the element according to its parent direction and set the radius with no logical properties

Hm, can we not just set the radius on the parent?

I'll see if this works out.
Thanks!

Comment 6

[Itiel]

Attachment: Bug 1673233 - Fix long text overflowing without ellipsis on the about:certificate tabs in RTL r?gijs,johannh

Comment 7

[Itiel]

(In reply to Itiel from comment #5)

3. To not regress 1671373 wrt the border-radius, target the element according to its parent direction and set the radius with no logical properties

Hm, can we not just set the radius on the parent?

I'll see if this works out.

It didn't :-(

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56697cdf8cf3
Fix long text overflowing without ellipsis on the about:certificate tabs in RTL r=johannh

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/56697cdf8cf3

Comment 10

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

The patch landed in nightly and beta is affected.
:itiel_yn8, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Itiel]

Comment on attachment 9183798 [details]
Bug 1673233 - Fix long text overflowing without ellipsis on the about:certificate tabs in RTL r?gijs,johannh

Beta/Release Uplift Approval Request

• User impact if declined: For RTL users, long about:certificate tab names can overflow off-view
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple patch, mostly minor CSS changes
• String changes made/needed: None

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9183798 [details]
Bug 1673233 - Fix long text overflowing without ellipsis on the about:certificate tabs in RTL r?gijs,johannh

83 visual regression for RTL users and the patch is small and CSS, uplift approved for 83 beta 8, thanks.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6691aed393c4