668813 - Arial font italic Arabic does not appear correctly

Status: RESOLVED FIXED

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

Description

[blinky]

Attachment: Arial font italic Arabic does not appear correctly

After converting the Arial font to italic 
Turns the Arial font to the font automatically  Traditional Arabic


Clarify:  http://i449.photobucket.com/albums/qq217/movh/9641e657.png

Comment 1

[Jonathan Kew [:jfkthame]]

You'll see similar behavior with Times New Roman, I expect.

The reason is that Arial (and Times) has an Italic face that's a separate font from the Regular face, _but_ the italic face does not include Arabic characters. So when you ask Firefox to display Arabic characters, we find the italic face of Arial (by matching the CSS font properties), but then we discover that it doesn't support the Arabic letters and so we fall back to a different font - either the next in the CSS font-family list, if provided, or a default from prefs.

This doesn't happen with Traditional Arabic because there is no separate italic face at all in the Trad Arab family, and so the Regular font is used with a synthetic "oblique" style.

I agree that it would be better to use synthetic-oblique characters from Arial instead of falling back to a different font family in this situation, but that's not currently how the font-matching algorithm operates.

I'm confirming this bug because although I believe the current behavior is "correct" as designed (and, I think, can be justified according to a strict interpretation of CSS2), I don't think it is really what users expect or want.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch, use synthetic italic rather than falling back to another family

This attempts to improve the behavior by checking for other faces in the chosen family that support the required character, before falling back to other families. I believe this brings our font-matching more into line with the CSS3 Fonts draft, as well as better meeting user expectations.

As noted in the code, a per-family character map could improve performance here (as well as in our existing fallback-search code); I'll put up a separate patch to implement that. (In practice, I doubt there's a measurable impact except perhaps on pages that have a LOT of fallback going on.)

Tryserver builds with this patch are available at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-78a18d8bec0c/.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: part 2 - build a per-family character map to optimize font matching

Comment 4

[Jonathan Kew [:jfkthame]]

John, review ping? It'd be nice to get this fixed before the next Aurora merge, as it affects quite a lot of Arabic-script pages.

Comment 5

[John Daggett (:jtd)]

Comment on attachment 543754 [details] [diff] [review]
part 2 - build a per-family character map to optimize font matching

Looks good.  But I would change 'ForgetCharacterMap' to 'ResetCharacterMap' or 'ClearCharacterMap'.

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to John Daggett (:jtd) from comment #5)
> Looks good.  But I would change 'ForgetCharacterMap' to 'ResetCharacterMap'
> or 'ClearCharacterMap'.

OK, it's now ResetCharacterMap.

Pushed to inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c433a1d53dd5
http://hg.mozilla.org/integration/mozilla-inbound/rev/431a8297db1f

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/c433a1d53dd5
http://hg.mozilla.org/mozilla-central/rev/431a8297db1f

Comment 9

[Jonathan Kew [:jfkthame]]

Part 2 was backed out in bug 678181 on suspicion of causing crashiness. The actual behavior change here for italic-styled Arabic should remain fixed, as that was implemented in part 1; we can consider re-landing the family-charmap optimization in a followup bug, once the crashiness is resolved.

Comment 10

[Jonathan Kew [:jfkthame]]

The part 2 backout did not resolve crashiness, only shifted the signatures slightly to show gfxFontFamily::FindFontForChar.

Hence part 1 was also backed out, until we figure out what's really going wrong. Re-opening.

Comment 11

[ebrahim]

Attachment: Firefox shot on data:text/html;charset=utf-8,<i>متن</i><div>متن</div>

Hi, I think my problem (that attached) have a relation to this bug.

Comment 12

[Jonathan Kew [:jfkthame]]

(In reply to ebraminio from comment #11)
> Created attachment 553692 [details]
> Firefox shot on data:text/html;charset=utf-8,<i>متن</i><div>متن</div>
> 
> Hi, I think my problem (that attached) have a relation to this bug.

Probably, but it also looks like you may have a non-Unicode-conformant font installed. I suggest you try the fontinfo addon (https://addons.mozilla.org/en-US/firefox/addon/fontinfo/), select the first line of text in your example, right-click on it and choose the Show Fonts in Selection command to find out exactly what font is being used there.

Comment 13

[ebrahim]

Attachment: The font used for rendering

Hmm, I attached some info that I hope be useful for solving this bug, because this bug (rendering italic Arial font in other font) annoying me also.
Thank you for your jobs.

Comment 14

[blinky]

When fixing this bug?

Comment 15

[ebrahim]

Just a little comment, this is a little testcase:
data:text/html;charset=utf-8,<div%20style="font-size:1000%"><i>%D9%85%D8%AC%D9%88</i><div>%D9%85%D8%AC%D9%88</div></div>
that shows all browsers failed on rendering italic font correctly. (safari is worst of them that hide italic text totally!)
I hope in near future Firefox render italic texts from fonts families that doesn't provide italic their-selves.

Comment 16

[(no longer active)]

Jonathan, what is the status of this bug?

Comment 17

[Jonathan Kew [:jfkthame]]

The patch here appeared to cause frequent crashes for some users, although I was never able to reproduce them; see bug 678181. I haven't been able to identify the problem by inspection, so I think the best chance of solving this may be to persuade someone who was able to reproduce the crashiness in bug 678181 to do some logging/debugging to try and understand what's going on.

I'll create a new tryserver build and see if we can get some insight.

Comment 18

[Jonathan Kew [:jfkthame]]

There's a tryserver build at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-421f48da1b08/ that includes the patch here (part 1 only, to keep things as simple as possible).

shaver, legneato: you both reported in bug 678181 that you were able to reproduce the crash while this was on trunk. Could you please try this build and see if it still reproduces?

Comment 19

[LegNeato]

I reproduced it with the try build. Hopefully I'll have time to run it under GDB today.

Comment 20

[LegNeato]

Actually, duh, I'll be on a plane. It'll have to wait for tomorrow :-)

Comment 21

[Jonathan Kew [:jfkthame]]

That'd be awesome! Offhand, details that might be helpful to have from GDB include:
- the specific character code for which FindFontForChar was called
- the complete Unicode string we're working on (e.g. from the gfxFontGroup::InitScriptRun frame, the characters in aString from aScriptRunStart to aScriptRunEnd)
- the font group's family-list string (mFamilies)
- the resolved fonts in the group (entries in the font group's mFonts array - you can get each one's name from its mFontEntry)

Of course, if there's an obvious problem that stands out when looking at it in the debugger, just say so! :)

Comment 22

[Jonathan Kew [:jfkthame]]

LegNeato: ping - if we could get some further details here, that'd be great.....

Comment 23

[Jonathan Kew [:jfkthame]]

Legneato, or anyone who was able to reproduce this - I'd love to get some traction here but need a bit of debugging help, at least to identify the problem fonts/characters so I can try to repro locally for further investigation.

Comment 24

[Jim Jeffery not reading bug-mail 1/2/11]

For the 2nd time today someone on IRC was asking to 'please fix' - 
I finally got him to give a couple of crash-reports, but since the build is without symbols, someone will have figure out and merge the dump to the sumbols...(so I was advised on IRC)

https://crash-stats.mozilla.com/report/index/bp-86baeaf5-7ea5-4172-b558-324ec2111203
https://crash-stats.mozilla.com/report/index/bp-58f11e0c-a36a-4305-8859-8721e2111203

Comment 25

[Jonathan Kew [:jfkthame]]

AFAICT, crash reports by themselves don't really help much - we had those in bug 678181 - what's needed is for someone who can reproduce the crash (seems to be dependent on the individual system, possibly installed fonts) to look at it under GDB and provide the info requested in comment #21.

I'd love to fix this, but I have no idea what's going wrong with the patches here that landed and were then backed-out for crashiness. I need some clues to help to identify the situation where those crashes happen, as I was never able to reproduce them on my system.

Comment 26

[LegNeato]

Shit, reproducing this under GDB fell off my plate. I'll get to it Monday, sorry!

Comment 27

[Jonathan Kew [:jfkthame]]

Thanks LegNeato, that'd be awesome.

I pushed a new tryserver build today based on current trunk + the patch from here, which you could use to confirm that the crashiness persists:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-35d63b9ed076/

Comment 28

[LegNeato]

Working on this now...

Comment 29

[LegNeato]

I can't get it to crash under gdb with a debugsymbols build built from tip of mozilla-central :-/

Comment 30

[Jonathan Kew [:jfkthame]]

(In reply to Christian Legnitto [:LegNeato] from comment #29)
> I can't get it to crash under gdb with a debugsymbols build built from tip
> of mozilla-central :-/

To clarify, when you say "tip of m-c", do you mean exactly that, or did you remember to apply the patch here (attachment 543740 [details] [diff] [review]) on top of that? This was backed out of m-c, so the crash isn't expected to show up unless you re-apply it locally.

Are you on the same system as a couple of months ago (comment #17), or made any major changes in the meantime?

Comment 31

[blinky]

Attachment: Arial font italic Arabic in Safari and Chrome on Mac OS X

I think the Mac OS X  does not support Arial font italic Arabic.

Even in other browsers, Example Safari and Chrome, Arial font italic Arabic does not appear correctly in the Mac OS X, and it becomes the Arial font italic Arabic to the font 'Geeza Pro'

Is it possible to apply the patch on the Windows operating system and other systems, except for the Mac OS X?

Comment 32

[Jonathan Kew [:jfkthame]]

(In reply to matjk from comment #31)
> Created attachment 584004 [details]
> Arial font italic Arabic in Safari and Chrome on Mac OS X
> 
> I think the Mac OS X  does not support Arial font italic Arabic.

The versions of Arial Italic I've seen, at least, do not include Arabic characters; what we want to do, therefore, is use the Arial Regular glyphs and slant them, rather than falling back to a different font altogether. The patch here implements this (in cross-platform code, so it is not specific to a platform).

> Is it possible to apply the patch on the Windows operating system and other
> systems, except for the Mac OS X?

No. The problem occurs on multiple platforms; the solution applies to multiple platforms; and when it was landed, we saw crashes on multiple platforms, too. So we really need to identify what was causing that, and fix the problem with this patch (or perhaps an underlying problem elsewhere that was exposed by this change).

Comment 33

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #30)
> (In reply to Christian Legnitto [:LegNeato] from comment #29)
> > I can't get it to crash under gdb with a debugsymbols build built from tip
> > of mozilla-central :-/
> 
> To clarify, when you say "tip of m-c", do you mean exactly that, or did you
> remember to apply the patch here (attachment 543740 [details] [diff] [review]
> [review]) on top of that? This was backed out of m-c, so the crash isn't
> expected to show up unless you re-apply it locally.
> 
> Are you on the same system as a couple of months ago (comment #17), or made
> any major changes in the meantime?

LegNeato, if you could confirm the answers here that would be helpful. In particular, if you're able to reproduce the crash with a Nightly build from, say, Aug 13th, but the same system can _not_ reproduce with tip + the patch from here (attachment 543740 [details] [diff] [review]), then I'd be inclined to speculate that some other change in the meantime may have fixed the problem, and re-land this patch to see whether the crashes reappear in nightly builds.

Comment 34

[blinky]

Attachment: bug in the arial font bold

After running the patch on Windows, I noticed a bug in the arial font bold.

Does not display arial font bold correctly sometimes.

Note, after disabling "Use hardware acceleration when available", Arial font bold  appears correctly.

Example site: http://www.adslgate.com

http://i449.photobucket.com/albums/qq217/movh/a920db27.png
http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1

http://i449.photobucket.com/albums/qq217/movh/3c99aaa3.png
http://www.adslgate.com/dsl/showpost.php?p=1063725009&postcount=3

Comment 35

[LegNeato]

(In reply to Jonathan Kew (:jfkthame) from comment #30)
> (In reply to Christian Legnitto [:LegNeato] from comment #29)
> > I can't get it to crash under gdb with a debugsymbols build built from tip
> > of mozilla-central :-/
> 
> To clarify, when you say "tip of m-c", do you mean exactly that, or did you
> remember to apply the patch here (attachment 543740 [details] [diff] [review]
> [review]) on top of that? This was backed out of m-c, so the crash isn't
> expected to show up unless you re-apply it locally.
> 
> Are you on the same system as a couple of months ago (comment #17), or made
> any major changes in the meantime?

Ah, I'm an idiot. Doing this now.

Comment 36

[LegNeato]

I built debug with the patch and tip of m-c:

http://pastebin.mozilla.org/1435832

I was unable to get it to crash running under gdb :-/. This machine is still set up the same, though I guess I could have installed updates (even Java?) recently...I don't recall doing so though.

I'll keep trying and will double check everything I built is as expected.

Comment 37

[LegNeato]

Ok, I can reproduce now:

1. Open graphwars.com/play.html
2. Once all the prompts and shit die down, open a tab with https://www.google.com/chrome?hl=ar

Boom.

I have a pastbin with some stuff here: http://pastebin.mozilla.org/1435951. 

Apparently my gdb is rusty and I am taking a fair amount of khuey's time. I'll keep fumbling, but here is the char looks like:

(gdb) p/c aMatchData.mCh
$6 = 39 '\''

I'll also see if I can narrow down the font now that I can reproduce 100% and know it is probably an arabic font.

Comment 38

[LegNeato]

err, http://graphwar.com/play.html that is. The other one is parked.

Comment 39

[John Daggett (:jtd)]

If this is a debug build you can stick in the code below to print the name/family:

    printf("font: %s family: %s\n", 
           NS_ConvertUTF16toUTF8(font->GetName()).get(),
           NS_ConvertUTF16toUTF8(family->Name()).get());

Insert that just before the line:

    family->FindFontForChar(&matchData);

Comment 40

[Jonathan Kew [:jfkthame]]

(In reply to Christian Legnitto [:LegNeato] from comment #37)

> I have a pastbin with some stuff here: http://pastebin.mozilla.org/1435951. 
> 
> Apparently my gdb is rusty and I am taking a fair amount of khuey's time.
> I'll keep fumbling, but here is the char looks like:
> 
> (gdb) p/c aMatchData.mCh
> $6 = 39 '\''

p/c isn't appropriate here, as aMatchData.mCh isn't a char, it's a Unicode character. According to the earlier "info locals" in your pastebin, it's actually 1575 (decimal) or U+0627, which is the Arabic letter ALEF.

Comment 41

[Jonathan Kew [:jfkthame]]

You could probably identify the font with something like

p/c *aMatchData->mFontToMatch->mFontEntry.mRawPtr->mName.mData@10

(where 10 is an arbitrary number of chars from the font name to be printed - looking at the .mLength field would tell you how many you really want).

Then I think I'd want to know whether aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily looks like a valid gfxFontFamily pointer, and what its mAvailableFonts array contains. (Note that it's OK for some of the mAvailableFonts entries to be NULL.)

Comment 42

[LegNeato]

Back on this today, trying the patch from comment 39

Comment 43

[LegNeato]

font: Courier family: Courier
font: Courier family: Courier
font: Courier family: Courier
font: LucidaGrande-Bold family: 
Invalid memory access of location 0x30 rip=0x101f5ac3b

($2 = {76 'L', 117 'u', 99 'c', 105 'i', 100 'd', 97 'a', 71 'G', 114 'r', 97 'a', 110 'n', 100 'd', 101 'e', 45 '-', 66 'B', 111 'o', 108 'l', 100 'd'

So our culprit is LucidaGrande-Bold with no family.

Comment 44

[LegNeato]

(gdb) print aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily
$1 = (gfxFontFamily *) 0x1196cd940

(gdb) print aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily->mAvailableFonts
$5 = {
  <nsTArray_base<nsTArrayDefaultAllocator>> = {
    mHdr = 0x0
  }, 
  <nsTArray_SafeElementAtHelper<nsRefPtr<gfxFontEntry>,nsTArray<nsRefPtr<gfxFontEntry>, nsTArrayDefaultAllocator> >> = {
    <nsTArray_SafeElementAtSmartPtrHelper<gfxFontEntry,nsTArray<nsRefPtr<gfxFontEntry>, nsTArrayDefaultAllocator> >> = {<No data fields>}, <No data fields>}, <No data fields>}

That probably wasn't the right way to print ha

Comment 45

[LegNeato]

User @Max_GH on twitter is also seeing these crashes, might be interesting to see what matches if we don't have enough info.

Comment 46

[Jonathan Kew [:jfkthame]]

(In reply to Christian Legnitto [:LegNeato] from comment #43)
> font: Courier family: Courier
> font: Courier family: Courier
> font: Courier family: Courier
> font: LucidaGrande-Bold family: 
> Invalid memory access of location 0x30 rip=0x101f5ac3b
> 
> ($2 = {76 'L', 117 'u', 99 'c', 105 'i', 100 'd', 97 'a', 71 'G', 114 'r',
> 97 'a', 110 'n', 100 'd', 101 'e', 45 '-', 66 'B', 111 'o', 108 'l', 100 'd'
> 
> So our culprit is LucidaGrande-Bold with no family.

OK, that's a step towards....something. It doesn't actually seem to be a font "with no family" (see below), but rather a font whose family is damaged somehow.

> (gdb) print aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily
> $1 = (gfxFontFamily *) 0x1196cd940

So the fontentry's mFamily pointer is not NULL (which is good, I suppose, as I can't figure out any way that we'd end up with a null family pointer); but you crash when trying to print the family's Name().

> (gdb) print aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily->mAvailableFonts
> $5 = {
>   <nsTArray_base<nsTArrayDefaultAllocator>> = {
>     mHdr = 0x0
>   }, 

Surely an array's mHdr pointer should never be null. In the case of an empty array, it should point to the static sEmptyHdr. This makes me suspect that either the font entry's mFamily pointer or the gfxFontFamily object itself must have been stomped on by something.

Comment 47

[John Daggett (:jtd)]

(In reply to Jonathan Kew (:jfkthame) from comment #46)
> > (gdb) print aMatchData->mFontToMatch->mFontEntry.mRawPtr->mFamily
> > $1 = (gfxFontFamily *) 0x1196cd940
> 
> So the fontentry's mFamily pointer is not NULL (which is good, I suppose, as
> I can't figure out any way that we'd end up with a null family pointer); but
> you crash when trying to print the family's Name().

I think I can imagine what's going on here.

Look at the ownership patterns in the code:

  gfxFontFamily ==> array of nsRefPtr<gfxFontEntry>

  gfxTextRun ==> gfxFontGroup ==> array of nsRefPtr<gfxFont>

  gfxFont ==> nsRefPtr<gfxFontEntry>

When gfxPlatformFontList::InitFontList is called it starts by clearing out the gfxFontFamily objects.  However, it doesn't clear out the text run cache or the font cache, so there's no guarantee that the gfxFontEntry objects are also cleared out, they may be hanging around due to indirect references from text runs or cached gfxFont objects.  But the mFamily pointer will now be a dangling reference which is, I suspect, what's happening here.

Seems like we need to explicitly clear the mFamily pointer within gfxFontEntry objects when deleting gfxFontFamily objects and not assume that it's non-null
within the code.

Comment 48

[Jonathan Kew [:jfkthame]]

(In reply to John Daggett (:jtd) from comment #47)

> When gfxPlatformFontList::InitFontList is called it starts by clearing out
> the gfxFontFamily objects.  However, it doesn't clear out the text run cache
> or the font cache, so there's no guarantee that the gfxFontEntry objects are
> also cleared out, they may be hanging around due to indirect references from
> text runs or cached gfxFont objects.  But the mFamily pointer will now be a
> dangling reference which is, I suspect, what's happening here.

Hmm.... it does seem like there's a potential problem there, but it's not clear to me that it explains the crashiness we've seen here. Normally, gfxPlatformFontList::InitFontList should be called at startup, and never called again unless we're notified of a change in the system's available fonts - but for the people who could reproduce these crashes, they had simple STR that didn't involve installing/removing fonts at the OS level.

> Seems like we need to explicitly clear the mFamily pointer within
> gfxFontEntry objects when deleting gfxFontFamily objects and not assume that
> it's non-null
> within the code.

Yes, we should do that. I don't think we assume non-null mFamily, we generally check for null before using it; but we do assume that if it's non-null, it's a valid gfxFontFamily, and that might not be true after a platform font change.

I'll file this separately, as it's a distinct, pre-existing bug.

Comment 49

[Jonathan Kew [:jfkthame]]

Filed bug 721315 on the possible dangling family pointers.

Speculating wildly here... it's interesting that Legneato's crash involved Lucida Grande, which - because it's the main UI font - would get a gfxFont instantiated early in startup, and probably sticks around in the cache permanently. Perhaps there's a scenario (depending on machine configuration in some way) whereby we (re-?)initialize the platform font list _after_ that first instance of Lucida Grande was created. If that happened, it'd make sense for the fallback-via-family patch here to lead to crashes.

Comment 50

[blinky]

Crash happens on Mac OS X 10.6 only

http://crash-stats.mozilla.com/report/index/bp-0eaad521-4720-4f70-9bfa-d87f02120126


Crash does not happen on Mac OS X 10.7 and 10.5

Comment 51

[Jonathan Kew [:jfkthame]]

I've pushed a new tryserver build with the patch here, as it's possible that bug 721315 (or other recent text/font changes) will have affected things. If people who were able to reproduce crashes with earlier versions could test this, it'd very much appreciated.

Builds are available at:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d83b2e47b061/

Comment 52

[blinky]

(In reply to Jonathan Kew (:jfkthame) from comment #51)
> I've pushed a new tryserver build with the patch here, as it's possible that
> bug 721315 (or other recent text/font changes) will have affected things. If
> people who were able to reproduce crashes with earlier versions could test
> this, it'd very much appreciated.
> 
> Builds are available at:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> d83b2e47b061/

Now I'm going to try this

Comment 53

[blinky]

(In reply to Jonathan Kew (:jfkthame) from comment #51)
> I've pushed a new tryserver build with the patch here, as it's possible that
> bug 721315 (or other recent text/font changes) will have affected things. If
> people who were able to reproduce crashes with earlier versions could test
> this, it'd very much appreciated.
> 
> Builds are available at:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> d83b2e47b061/

OK, after testing this, crash did not happen.

Comment 54

[blinky]

Attachment: bug in the firefox arabic version on Windows 7, italic arabic font in interface

I noticed a bug in the Arabic version on Windows 7, italic arabic font in interface does not appear correctly.

Comment 55

[ebrahim]

https://bugzilla.mozilla.org/show_bug.cgi?id=668813#c11 is reproducible again with this try-build for me

Comment 56

[Jonathan Kew [:jfkthame]]

Both matjk and ebraminio, regarding the problem with italic Arabic showing as junk: please confirm
(a) whether hardware acceleration is enabled on your system (check the Graphics section in about:support), and
(b) have you made any change to the preference gfx.font_rendering.harfbuzz.scripts (in about:config) from its default value of 3?

Comment 57

[ebrahim]

(a). No, because "GPU Accelerated Windows0. Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer." (I will update my graphic driver soon)
(b). No

Comment 58

[blinky]

Attachment: testcase

testcase, Arial font italic Arabic

Comment 59

[ebrahim]

After enabling gfx.direct2d.force-enabled, bug fixed for me! Thanks!

Comment 60

[blinky]

Attachment: testcase

testcase

Comment 61

[ebrahim]

I updated my GPU, so now without gfx.direct2d.force-enabled my Firefox have "GPU Accelerated Windows 1/1 Direct3D 10" but before update, my Firefox had ugly font anti-alias/smoothing (May that is why my old GPU driver was blacklist-ed), I hope you fix not supported GPUs bug. Thanks :)

Comment 62

[blinky]

Attachment: testcase

Comment 63

[Jonathan Kew [:jfkthame]]

(In reply to ebraminio from comment #59)
> After enabling gfx.direct2d.force-enabled, bug fixed for me! Thanks!

Aha - that's helpful to know, thanks. One more thing: if you set gfx.direct2d.force-enabled to FALSE again, but then set gfx.font_rendering.directwrite.enabled to TRUE (and restart the browser), does the italic Arabic render correctly?

Comment 64

[blinky]

Attachment: testcase

testcase

Comment 65

[ebrahim]

Sorry, as I said (on #c59), I have updated my GPU Driver (I can not do that without reinsatlling my Windows!), so I can not do that because Direct2d is enable! So I did other thing, I turned gfx.direct2d.disabled to true and Direct2d disabled for me (note that now I have a compatible driver, but I manually disabled Direct2d), after disabling Direct2d, bug appeared again! After that, I turned gfx.font_rendering.directwrite.enabled to true and bug fixed!

So, in a nutshell, if you turn this `gfx.direct2d.disabled` to true, [I guess] you can see our bug (data:text/html;charset=utf-8,<i>متن</i><div>متن</div>) and if you turn `gfx.font_rendering.directwrite.enabled` to true this bug will fix.

I hope this information be helpful for you. Thank you for your good works :)

Comment 66

[blinky]

(In reply to Jonathan Kew (:jfkthame) from comment #56)
> Both matjk and ebraminio, regarding the problem with italic Arabic showing
> as junk: please confirm
> (a) whether hardware acceleration is enabled on your system (check the
> Graphics section in about:support), and
> (b) have you made any change to the preference
> gfx.font_rendering.harfbuzz.scripts (in about:config) from its default value
> of 3?

After enable hardware acceleration solved the problem, But before you apply this patch bug do not exist.

Comment 67

[blinky]

(In reply to ebraminio from comment #11)
> Created attachment 553692 [details]
> Firefox shot on data:text/html;charset=utf-8,<i>متن</i><div>متن</div>
> 
> Hi, I think my problem (that attached) have a relation to this bug.

You use the channel nightly-ux, this version is under development and unstable.

Better to use the channel release, this version is stable.

Download the stable version of Firefox http://www.mozilla.com/firefox

Comment 68

[Jonathan Kew [:jfkthame]]

OK, I understand what's happening with the garbled italic text. This is actually a separate bug that already exists in the code, but becomes easier to trigger once the patch here is applied.

I've filed bug 724231 about that issue; we need to resolve that before we can deploy the fix here, otherwise this will make things worse instead of better for users in GDI-font environments (WinXP, and Win7 without hardware acceleration).

Comment 69

[blinky]

Character spacing is different in Firefox compared to other browsers, for example Chrome.

Example site: http://news.softpedia.com/friday


In Firefox
http://i449.photobucket.com/albums/qq217/movh/d207a888.png

In Chrome
http://i449.photobucket.com/albums/qq217/movh/4e002254.png


Is this a bug in Firefox?

Comment 70

[Jonathan Kew [:jfkthame]]

(In reply to matjk from comment #69)
> Character spacing is different in Firefox compared to other browsers, for
> example Chrome.

Firefox by default applies kerning defined in the font; Chrome doesn't, AFAIK. So that could explain differences, especially with character pairs like "TA".

Actually, spacing looks pretty bad in _both_ those screenshots (e.g. look at "ICS" in the fourth headline.)

In any case, this is not relevant to this bug. If there's an issue (aside from the known difference regarding kerning), it should be filed separately.

Comment 71

[blinky]

(In reply to Jonathan Kew (:jfkthame) from comment #70)
> (In reply to matjk from comment #69)
> > Character spacing is different in Firefox compared to other browsers, for
> > example Chrome.
> 
> Firefox by default applies kerning defined in the font; Chrome doesn't,
> AFAIK. So that could explain differences, especially with character pairs
> like "TA".

You're right

This shows clearly in the word "AVAST" In the link http://translate.google.com/?hl=en&tab=wT#auto|fr|AVAST

Thanks

Comment 72

[Jonathan Kew [:jfkthame]]

Re-landed these patches (with pt 2 rebased for PRBool->bool change) on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa0c78177fb9
https://hg.mozilla.org/integration/mozilla-inbound/rev/80fa4ccd0b2b

We'll need to watch for the reappearance of crashes like bug 678181, but I'm currently assuming (based on comment 53) that bug 721315 has resolved that problem, so that this fix will now be safe.

Comment 73

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/fa0c78177fb9
https://hg.mozilla.org/mozilla-central/rev/80fa4ccd0b2b

Comment 74

[blinky]

After applying the patch, I discovered a bug exists.


Times New Roman Bold font Arabic turns to Arial font.

Example site: http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1


After applying the patch
http://i449.photobucket.com/albums/qq217/movh/da6aa65b.png

Before applying the patch
http://i449.photobucket.com/albums/qq217/movh/b14b9f7e.png


Should not turn Times New Roman Bold font Arabic to Arial font.


In Comment 34, I mentioned that bug in the Arial font, now discovered that the bug is in Times New Roman Bold font Arabic font.

Comment 75

[blinky]

see the word "Radeon" in http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1

The word "Radeon" appears in Arial Black font.

http://i449.photobucket.com/albums/qq217/movh/d82e6a66.png


Should show the word "Radeon" in Arial Bold font instead of Arial Black, Example http://i449.photobucket.com/albums/qq217/movh/29f88a76.png

Comment 76

[Jonathan Kew [:jfkthame]]

(In reply to matjk from comment #75)
> see the word "Radeon" in
> http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1
> 
> The word "Radeon" appears in Arial Black font.
> 
> http://i449.photobucket.com/albums/qq217/movh/d82e6a66.png
> 
> 
> Should show the word "Radeon" in Arial Bold font instead of Arial Black,
> Example http://i449.photobucket.com/albums/qq217/movh/29f88a76.png

No, Arial Black is correct here, because that title is in a <b> (bolder) element, within a <div> that is already styled with font-weight:bold. The computed value for font-weight is therefore 900, and the best match is Arial Black.

I'm guessing that your screenshot that shows Arial Bold was made under GDI (i.e. with hardware acceleration disabled), where the Arial Black face is not recognized as belonging to the Arial family, whereas the new one has HWA (or at least DirectWrite) enabled. This affects the availability and organization of font faces.

Comment 77

[Jonathan Kew [:jfkthame]]

(In reply to matjk from comment #74)
> After applying the patch, I discovered a bug exists.
> 
> Times New Roman Bold font Arabic turns to Arial font.
> 
> Example site:
> http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1
> 
> After applying the patch
> http://i449.photobucket.com/albums/qq217/movh/da6aa65b.png
> 
> Before applying the patch
> http://i449.photobucket.com/albums/qq217/movh/b14b9f7e.png
> 
> Should not turn Times New Roman Bold font Arabic to Arial font.

This seems very strange - could you create a minimal testcase that shows the behavior, and file a new bug about it, please?

Comment 78

[blinky]

(In reply to Jonathan Kew (:jfkthame) from comment #77)
> (In reply to matjk from comment #74)
> > After applying the patch, I discovered a bug exists.
> > 
> > Times New Roman Bold font Arabic turns to Arial font.
> > 
> > Example site:
> > http://www.adslgate.com/dsl/showpost.php?p=1063716613&postcount=1
> > 
> > After applying the patch
> > http://i449.photobucket.com/albums/qq217/movh/da6aa65b.png
> > 
> > Before applying the patch
> > http://i449.photobucket.com/albums/qq217/movh/b14b9f7e.png
> > 
> > Should not turn Times New Roman Bold font Arabic to Arial font.
> 
> This seems very strange - could you create a minimal testcase that shows the
> behavior, and file a new bug about it, please?

I created a new report in Bug 728518

Thanks