Closed Bug 382542 Opened 13 years ago Closed 12 years ago

Font entries need to depend on the style (Arabic characters not displayed in italic)

Categories

(Core :: Graphics, defect, P1)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: pavlov, Assigned: pavlov)

References

()

Details

Attachments

(8 files, 5 obsolete files)

Given something like Arial with italic style the Windows font mapper creates a font with the Arial Italic font face.  We think we're using Arial and check its character set which is different than Arial's character set.  We need to tell which font we're actually using (or do the mapping ourselves early on) so that we can check the right character set.
Flags: blocking1.9+
Target Milestone: --- → mozilla1.9alpha6
After discussing with Stuart, moving to B1.
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1
Blocks: 334728
Priority: -- → P2
Target Milestone: mozilla1.9 M8 → ---
Summary: Font entries need to depend on the style → Font entries need to depend on the style (Arabic characters not displayed in italic)
you can also see this using the STIXNonUnicode fonts where they use a different PUA codepoint for each style (i.e. U+0E261-U+0E264 are all "stix-old style digit 0")
Blocks: 400938
Duplicate of this bug: 404953
Blocks: 324857
Blocks: 413115
Blocks: 416062
No longer blocks: 416062
Priority: P2 → P1
Stuart, I heard a rumor you might be working on this.  How's it coming?
Hoping to have it done this week.  It can wait for RC1 imho.
Priority: P1 → P2
Blocks: 419744
Flags: tracking1.9+ → blocking1.9+
No longer blocks: 324857
Attached patch v0.6 (obsolete) — Splinter Review
this works.  it ends up adding a lot of font variations that we might not need to our font list.  i suspect this stinks up startup time a bit.  probably doesn't make much difference for Tp.
Attached patch v0.75 (obsolete) — Splinter Review
this works a lot better. still need to make pref font caches depend on style as well as move weight selection over to use this new code.
Attachment #310082 - Attachment is obsolete: true
Attached patch v0.8 (obsolete) — Splinter Review
ok, this looks pretty good.  still need to make cached prefs depend on styles as well as take a look at the bad underline stuff as it looks to be calling FindFontEntry without a style still.
Attachment #310165 - Attachment is obsolete: true
shifting to P1.  This needs to go in to beta 5 -- the patch is getting big.
Priority: P2 → P1
Attached patch v1.0 (obsolete) — Splinter Review
Attachment #310199 - Attachment is obsolete: true
Attachment #310341 - Flags: review?(vladimir)
Attached patch v1.05Splinter Review
fixes a problem with InitBadUnderlineList using FindFontEntry without a style.
Attachment #310341 - Attachment is obsolete: true
Attachment #310349 - Flags: review?(vladimir)
Attachment #310341 - Flags: review?(vladimir)
Comment on attachment 310349 [details] [diff] [review]
v1.05

This doesn't look too scary; I gave a few comments over irc.  Make sure all reftests etc. pass?
Attachment #310349 - Flags: review?(vladimir) → review+
Arabic is rendered strangely in italics in some situations.  Times New Roman goes all hexen with italics applied.  Verdana, a font with no Arabic glyphs, switches fallback font *family* when italics are applied, looks very strange.  Prior to the patch, the hex boxes under italics would *always* occur, after the patch it only happens for certain fonts.

Not sure this is super important, italics is not a typographic style natural to Arabic, especially given the common tendency for Arabic glyphs to slant left in normal style faces.
1. Copy Hiragino Maru Gothic Pro from a Mac system:

find /System/Library/Fonts/ -name "*Pro*.otf" -print
/System/Library/Fonts//ヒラギノ丸ゴ Pro W4.otf
/System/Library/Fonts//ヒラギノ明朝 Pro W3.otf
/System/Library/Fonts//ヒラギノ明朝 Pro W6.otf
/System/Library/Fonts//ヒラギノ角ゴ Pro W3.otf
/System/Library/Fonts//ヒラギノ角ゴ Pro W6.otf

The "Maru Gothic" face is the first one in the list above.

2. Run the attached testcase

Results:

- Hiragino Maru Gothic Pro doesn't bold until "bolder 5" 
==> should be bolder 1 and beyond

- Hiragino Maru Gothic Pro doesn't lighten until "lighter 4" 
==> should be lighter 1 and beyond

- MS Gothic *never* bolds at any weight!

FF2/Win renders correctly.

Note: this was tested after applying the fontselection.diff patch.
Left:  bold fonts are displayed correctly
Right: bold fonts missing (in webpage contents & in tab names on tabbar)
Target Milestone: --- → mozilla1.9beta5
Attached patch v1.0 (part 2) (obsolete) — Splinter Review
This should fix all the problems that john daggett was seeing.  I've changed the search all fonts code to use the same code as everything else for selecting a font face from a family.  This will yield better results and fix john's problem.

I also fixed the bold problem he was seeing.  We should probably support synthetic bold when there are no other bold faces, but we can do that in another bug.
Attachment #310523 - Flags: review?(vladimir)
Comment on attachment 310523 [details] [diff] [review]
v1.0 (part 2)

Looks fine, get John to look at it also?
Attachment #310523 - Flags: review?(vladimir) → review+
Attached patch v1.05 (part 2)Splinter Review
slight update to the previous patch
Attachment #310523 - Attachment is obsolete: true
Attachment #310635 - Flags: superreview?(jdaggett)
Attachment #310635 - Flags: review+
Comment on attachment 310635 [details] [diff] [review]
v1.05 (part 2)

Looks fine.
Attachment #310635 - Flags: superreview?(jdaggett) → superreview+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #18)
> regression ?
> 
> see http://forums.mozillazine.org/viewtopic.php?p=3301380#3301380
> 

20080319_1911_firefox-3.0b5pre.en-US.win32.zip

not fixed.
bold is not bold.
Attached patch bold fixSplinter Review
This adds FontEntrys for fake bold items so we'll use 600 weight when we don't have anything better.
Attachment #310687 - Flags: review?(vladimir)
20080319_2244_firefox-3.0b5pre.en-US.win32.zip

fixed, thanks.
Depends on: 424018
Depends on: 424165
Duplicate of this bug: 424126
Duplicate of this bug: 393439
You need to log in before you can comment on or make changes to this bug.