Closed Bug 1458158 Opened 6 years ago Closed 6 years ago

Bold font not rendering for Arabic characters when the style calls for Arial with font-weight:900

Categories

(Core :: Layout: Text and Fonts, defect, P1)

61 Branch
x86_64
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: over68, Assigned: jfkthame, NeedInfo)

References

Details

(Keywords: regression)

Attachments

(4 files)

Bold font not rendering for Arabic characters when the style calls for Arial with font-weight:900.

Testcase https://bugzilla.mozilla.org/attachment.cgi?id=598489
Flags: needinfo?(jfkthame)
This similar to bug 728518.
What result do you get for the testcase? (Could you attach a screenshot, please.)
Flags: needinfo?(jfkthame) → needinfo?(over68)
Ah, never mind about a screenshot, I could reproduce this on a Win10 machine. So yes, the updated font code has regressed bug 728518. Will fix; thanks for the report!
Flags: needinfo?(over68)
Here's a reftest form of the testcase; this currently fails on trunk due to the bug here.
Attachment #8972303 - Flags: review?(jwatt)
Assignee: nobody → jfkthame
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Fortunately the fix is simple: in the update to CalcStyleMatch, I inadvertently reversed the sense of the weight factor, so that it prefers further weights instead of closer ones. Fixing that restores the desired behavior. (Actually, I think as a followup, we should consider replacing CalcStyleMatch entirely, and instead relying on the same WeightStyleStretch distance function as the main font selection code. But let's deal with that separately, as it could change behavior -- for the better, I think -- as opposed to simply fixing the regression here.)
Attachment #8972304 - Flags: review?(jwatt)
Severity: normal → critical
Priority: -- → P1
(In reply to Jonathan Kew (:jfkthame) from comment #7)
> (Actually, I think as a followup, we should consider replacing
> CalcStyleMatch entirely, and instead relying on the same WeightStyleStretch
> distance function as the main font selection code. But let's deal with that
> separately, as it could change behavior -- for the better, I think -- as
> opposed to simply fixing the regression here.)

Can you file a bug for that and mention the bug number here?
Comment on attachment 8972303 [details] [diff] [review]
Testcase for rendering Arabic characters when styled with an extra-bold weight of Arial

Review of attachment 8972303 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/font-matching/1458158-1-ref.html
@@ +14,5 @@
> +    h1 {
> +        font-family: arial_bold;
> +    }
> +    b {
> +        font-family: arial_black, arial_bold;

The arial_bold here can go, right?
Attachment #8972303 - Flags: review?(jwatt) → review+
Attachment #8972304 - Flags: review?(jwatt) → review+
(In reply to Jonathan Watt [:jwatt] from comment #9)
> Comment on attachment 8972303 [details] [diff] [review]
> Testcase for rendering Arabic characters when styled with an extra-bold
> weight of Arial
> 
> Review of attachment 8972303 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/reftests/font-matching/1458158-1-ref.html
> @@ +14,5 @@
> > +    h1 {
> > +        font-family: arial_bold;
> > +    }
> > +    b {
> > +        font-family: arial_black, arial_bold;
> 
> The arial_bold here can go, right?

No, it's there because Arial Black does not actually include any Arabic characters - it has a reduced character set. And so at font-weight:900, although we initially choose the Black face, we want them to fall back to Arial Bold (and not to Regular, as they did thanks to this bug, or to something else entirely).

So it's explicitly here to provide a controlled fallback for those characters.
(In reply to Jonathan Watt [:jwatt] from comment #8)
> (In reply to Jonathan Kew (:jfkthame) from comment #7)
> > (Actually, I think as a followup, we should consider replacing
> > CalcStyleMatch entirely, and instead relying on the same WeightStyleStretch
> > distance function as the main font selection code. But let's deal with that
> > separately, as it could change behavior -- for the better, I think -- as
> > opposed to simply fixing the regression here.)
> 
> Can you file a bug for that and mention the bug number here?

Filed as bug 1458301.
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74b436c29aa1
Testcase for rendering Arabic characters when styled with an extra-bold weight of Arial. r=jwatt
https://hg.mozilla.org/integration/mozilla-inbound/rev/7faed86e2815
Correct the weight-difference factor in CalcStyleMatch to favor closer weights, not further ones. r=jwatt
https://hg.mozilla.org/mozilla-central/rev/74b436c29aa1
https://hg.mozilla.org/mozilla-central/rev/7faed86e2815
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Thanks for landing an automated test for this. Is this something we should have QE do some extra exploratory testing around or are you reasonably confident that the new test will catch any issues?
Flags: qe-verify?
Flags: needinfo?(jfkthame)
Flags: in-testsuite+
Actually, I'm going to say that the answer to that question is yes since the new test only runs on Windows.
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(jfkthame)
Additional testing wouldn't hurt, but the issue is one that we've only ever seen on Windows; it arises because of an oddity of the particular collection of Arial font faces that Windows ships, and the way those are organized by the DirectWrite font subsystem.

In theory a similar issue could occur on other platforms, but we've never seen an actual case AFAIK. (It would require creating a deliberately anomalous font family with multiple weights and differing character repertoires across the weights.)
I reproduced this issue on Windows 10 x64, Windows 7 x64 and Mac OS X 10.12 using Nightly 61.0a1(2018-04-30).
I could not reproduce it on Ubuntu 17.04 x64 using Nightly 61.0a1(2018-04-30) and the latest Nightly 61.0a1(2018-05-03).

The issue is verified fixed on Windows 10 x64, Windows 7 x64 and Mac OS X 10.12a using the latest Nightly 61.0a1(2018-05-03).

Also, I tried to reproduce the initial issue on Windows 10 x64 and Windows 7 with an affected Nightly 61.0a1 build (2018-04-30) using several websites with Arabic characters and Arial bold fonts, without luck.

Is there any testing that I should do in order to verify this bug?
Flags: needinfo?(jfkthame)
The problem occurs in affected builds when Arabic characters are used with Arial weight 900 (extra-bold/black), not with the normal Bold face. So for example if a site has a heading <h1> which is bold by default, and then uses <b> within the heading, the resolved font-weight will be 900 and the bug will occur.

The older report bug 728518 mentioned in comment 2 had a link to https://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1 where the issue occurs: the Arabic text of the blue heading should appear in Arial Bold, but this bug caused it to appear in Arial (regular) instead.

Verifying with the attached testcase should be OK, however.
Flags: needinfo?(jfkthame)
Attached image Arial bold arabic.png
I could not reproduce the bug using the test case from comment 18 on Windows 10 x64, Mac OS X 10.12 and Ubuntu 17.04 x64 with an older Nightly 61.0a1 (2018-04-30) and with the latest Nightly 61.0a1 (2018-05-06).

Am I missing something?
Flags: needinfo?(jfkthame)
Your screenshot (comment 19) does actually show the issue, but it's difficult to see at such a small font size. If we put the two lines of text directly above each other, as here, it becomes clearer: the top line (from Nightly 04-30) does not use the same font face for the Arabic script as the lower line (from 05-06).

What we're seeing here is that the bottom line correctly uses Arial Bold for the Arabic script text, but the upper line is (I think) using the Arial Regular face with a synthetic-bold effect. The difference is particularly visible on the two- and three-dot combinations above some of the letters such as in the right-most word شركة.
Flags: needinfo?(jfkthame)
Based on comment 17 and comment 20 I will mark this bug as Verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.