1578906 - Underlined single-characters (accesskeys) in Firefox Preferences are only barely underlined, with text-decoration-skip-ink

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Attachment: screenshot of issue with "Cryptominers" underlining

STR:

1. In Firefox Nightly, visit about:preferences.
2. In the "Privacy and Security" section, under Enhanced Tracking Protection, click "Custom"
3. Look at the third checkbox, which says "Cryptominers".

EXPECTED RESULTS: The "y" should be clearly underlined, to indicate that it's an accesskey or something.

ACTUAL RESULTS: The "y" is meant to be underlined, but due to text-decoration-skip-ink, it looks more like a tiny underscore between the "y" and the "p"

The same issue is visible in the "OCSP" section at the very bottom of "Privacy and Security". There is a label "Query OCSP [etc]" where the "Q" is underlined, and the descender intersects the underline position so that the underline just kinda looks like an underscore that partially sticks underneath the Q.

SUGGESTED FIX: Adjust the rule for span.accesskey in xul.css to add text-decoration-skip-ink:none, to turn off ink-skipping entirely.

• Pro: this will draw a full underline and restore the preexisting look for this styling before.
• Con: This will make the underline overlap the descender which might look kinda broken. However, we've been OK with this look in Firefox builds up until now, and I think it looks less-broken than it currently looks with the ink-skipping. Ink-skipping really only makes sense when you're underlying a whole word-phrase -- not a single character.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: Screenshot of issue with "Query" underlining

Comment 2

[Adolfo Jayme]

First of all, this is a consequence of exercising poor judgment in choosing the accelerator keys—every single software localizer knows that you shouldn’t choose letters with descenders (that’s why “_Help” is “Ay_uda” and not “A_yuda” in Spanish, for example). But there is a programmatic fix that doesn’t require going through every single affected string, and will avoid the need of using text-decoration-skip-ink:none: implement a character blacklist. This is what the Chromium team did: it made it so that the characters /, \ and _ are never ink-skipped, to avoid this exact problem.

Comment 3

[Adolfo Jayme]

OTOH, the GTK toolkit just draws the underline in a lower position when an accelerator is used in a letter with a descender.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Yeah I think this makes sense since we are only drawing the underline for one character.

Comment 5

[Paulina]

I would like to take the case, I've already tested the suggested solution in my local environment.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Yes you may work on it. I will assign it to you when you upload a patch. Please comment here or join #fx-team or #introduction on Mozilla's IRC if you have any questions.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

I'm assigning it to you now since someone told me you might not be able to upload an attachment if you're not assigned.

Comment 8

[Paulina]

Thanks Jared, it is actually my first contribution (ever) , so I have a lot to understand... I'm joining the #introduction irc channel that you've recommended before and will ask for help .

Comment 9

[Arieski]

Hi, I would like to work on this :)

Comment 10

[aarushivij]

Hello, Can I submit the patch for this issue?
Thanks :)
Aarushi

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Yes, you may work on this. Whoever wants to work on this can, I will mark it as assigned once there is a patch uploaded.

Comment 12

[aarushivij]

Attachment: Bug 1578906 - Show a larger underline on the accesskeys since they are only single characters and descenders can make the underline appear very small for an accesskey.

Comment 13

[aarushivij]

Attachment: afterEdit.png

After the fix :)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b52e6c5ce1b
Show a larger underline on the accesskeys since they are only single characters and descenders can make the underline appear very small for an accesskey. r=jaws

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4b52e6c5ce1b

Comment 16

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue using Firefox 71.0a1 (20190904163258) on Windows 10x64.
The issue is verified fixed with Firefox 76.0b4 (20200412214314) on Windows 10x64 and Ubuntu 18.04.