Closed Bug 1653899 Opened 2 months ago Closed 1 month ago

Zap underline doesn't work when spanning multiple lines or with longer text.

Categories

(Firefox :: Messaging System, defect, P1)

defect

Tracking

()

VERIFIED FIXED
82 Branch
Iteration:
82.1 - Aug 24 - Sep 6
Tracking Status
firefox79 --- unaffected
firefox80 --- wontfix
firefox81 --- verified
firefox82 --- verified

People

(Reporter: flod, Assigned: emcminn)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

Attached image immagine.png

This is the current Italian translation

onboarding-multistage-import-header = Importa password, segnalibri e <span data-l10n-name="zap">altro ancora</span>

Notice how the zap doesn't follow on the second line.

I can work around it by adding a non-breaking space, but I think it should be fixed on the code side. I tried adding a white-space: nowrap, and turns out the highlight it's too short for 2 words, although they're not very long.

Attached image Screenshot with nowrap
Assignee: nobody → emcminn
Severity: -- → S3
Iteration: --- → 80.2 - July 13 - July 26
Priority: -- → P1

NI Flod to confirm if translation can be edited to drop 'ancora' and if 'Importa password, segnalibri e altro' suffice to resolve this issue. Thanks

Flags: needinfo?(francesco.lodolo)

(In reply to Punam Dahiya [:pdahiya] from comment #2)

NI Flod to confirm if translation can be edited to drop 'ancora' and if 'Importa password, segnalibri e altro' suffice to resolve this issue. Thanks

That's not a solution, because I might have control over Italian, but that doesn't scale for ~90 languages.

Flags: needinfo?(francesco.lodolo)

(In reply to Francesco Lodolo [:flod] from comment #0)

Created attachment 9164643 [details]
immagine.png

This is the current Italian translation

onboarding-multistage-import-header = Importa password, segnalibri e <span data-l10n-name="zap">altro ancora</span>

Notice how the zap doesn't follow on the second line.

Zap not breaking to new line is known issue. See tips in https://protocol.mozilla.org/patterns/atoms/zap.html#zap

I can work around it by adding a non-breaking space, but I think it should be fixed on the code side. I tried adding a white-space: nowrap, and turns out the highlight it's too short for 2 words, although they're not very long.

Adding white-space: nowrap to enclosing tag and using aspect ratio for svg helping it scale better for wider texts should be the fix here.

Indonesian (id) definitely needs two words "yang lain".
Readability is poor because overlap between descenders and zap. (letter "y" and "g")

Thanks for further examples - we're also intending for the descenders in the text to sit "on top" of the zap, which should improve readability.

Iteration: 80.2 - July 13 - July 26 → 81.1 - July 27 - Aug 09

We've chosen a different (longer) zap that, along with the nowrap, should fix this issue.

Iteration: 81.1 - July 27 - Aug 09 → 81.2 - Aug 10 - Aug 23
Attached image welcome.png
Comment on attachment 9169681 [details]
welcome.png

Work well with 81.0a1 (2020-08-12) (compiled with official branding)

(In reply to fabrixx2 from comment #10)

Comment on attachment 9169681 [details]
welcome.png

Work well with 81.0a1 (2020-08-12) (compiled with official branding)

Italian localization has a non-breaking space at the moment, so not exactly relevant for testing.

Attachment #9166577 - Attachment description: Bug 1653899 - New zap .svg to allow for longer spans → Bug 1653899 - New long zap .svg to allow for longer spans; plus dynamic measuring
Iteration: 81.2 - Aug 10 - Aug 23 → 82.1 - Aug 24 - Sep 6
Pushed by emcminn@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dd306951f80c
New long zap .svg to allow for longer spans; plus dynamic measuring r=pdahiya
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Do we need this on Beta81 or can this ride the 82 train to release?

Flags: needinfo?(emcminn)

Hi Ryan, we'd like to uplift this, once it's verified by QA. Thanks!

Flags: needinfo?(emcminn)

Hi Robert, can we can QA verification for this patch since we would like to uplift it? Thanks!

Flags: needinfo?(romartin)

I have verified that the "Zap underline" is correctly displayed under multiple lines and under longer text using Firefox Nightly 82.0a1 en-US, DE, FR, RU, and IT (Build ID: 20200831215215) on Windows 10 x64, macOS 10.15, and Ubuntu Linux 18.04.

Status: RESOLVED → VERIFIED
Flags: needinfo?(romartin)

Comment on attachment 9166577 [details]
Bug 1653899 - New long zap .svg to allow for longer spans; plus dynamic measuring

Beta/Release Uplift Approval Request

  • User impact if declined: Currently, the "zap" decoration on about:welcome doesn't display correctly on longer strings; which will affect primarily non-en/US locales.
  • 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): This is a design change only; it touches the "zap" assets (svgs) and their render method.
  • String changes made/needed: none
Attachment #9166577 - Flags: approval-mozilla-beta?

Comment on attachment 9166577 [details]
Bug 1653899 - New long zap .svg to allow for longer spans; plus dynamic measuring

Approved for 81.0b5.

Attachment #9166577 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have verified that the "Zap underline" is correctly displayed under multiple lines and under longer text using Firefox Beta 81.0b5 en-US, DE, FR, RU, and IT (Build ID: 20200901203141) on Windows 10 x64, macOS 10.15, and Ubuntu Linux 18.04.

You need to log in before you can comment on or make changes to this bug.