Open Bug 390740 Opened 12 years ago Updated Last year

Pass lang-group into GetMetricsFor more consistently

Categories

(Core :: Layout, defect)

x86
Windows XP
defect
Not set

Tracking

()

REOPENED

People

(Reporter: sharparrow1, Assigned: sharparrow1)

Details

(Whiteboard: [not needed for 1.9])

Attachments

(2 files)

Getting the right font metrics for a font depends on getting the right lang-group; otherwise, we're likely to pick the wrong font, especially for CJK languages.

The way I'm going about this is calling the helpers in nsLayoutUtils more consistently; these helpers do the right thing.
Attached patch General patchSplinter Review
I also have some changes in my tree to cover SVG, MathML, and style/; I'll wait for review on this before I post them, though.
Attachment #275069 - Flags: review?(roc)
Comment on attachment 275069 [details] [diff] [review]
General patch

These helper functions should probably return already_Addrefed<nsIFontMetrics>... or even, eventally, already_Addrefed<gfxFontMetrics>. But not necessary now.
Attachment #275069 - Flags: superreview+
Attachment #275069 - Flags: review?(roc)
Attachment #275069 - Flags: review+
Attachment #275069 - Flags: approval1.9?
Attached patch PatchSplinter Review
Here are the SVG changes (small patch, but I figure an SVG peer should look at it).
Attachment #275296 - Flags: superreview?(roc)
Attachment #275296 - Flags: review?(tor)
I guess I should comment on the risk for this patch: it should be very low-risk, because the helper methods do basically the same thing as the current code, except that they pass in the right lang-group.
Attachment #275296 - Flags: review?(tor) → review+
Comment on attachment 275069 [details] [diff] [review]
General patch

a1.9=dbaron
Attachment #275069 - Flags: approval1.9? → approval1.9+
Comment on attachment 275069 [details] [diff] [review]
General patch

Checked in "General patch".
Attachment #275296 - Flags: superreview?(roc) → superreview+
Attachment #275296 - Flags: approval1.9?
Comment on attachment 275296 [details] [diff] [review]
Patch

a1.9=dbaron
Attachment #275296 - Flags: approval1.9? → approval1.9+
SVG fix checked in.
These patches were both checked-in... resolving.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I'm not sure if that means that this was finished, though.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
is this done? can someone mark it fixed if so?
Comment on attachment 275069 [details] [diff] [review]
General patch

Resetting all approval1.9+ flags on bugs that have not been checked in by Oct 22 11:59 PM PDT.  Please re-request approval if needed.
Attachment #275069 - Flags: approval1.9+
Comment on attachment 275296 [details] [diff] [review]
Patch

Resetting all approval1.9+ flags on bugs that have not been checked in by Oct 22 11:59 PM PDT.  Please re-request approval if needed.
Attachment #275296 - Flags: approval1.9+
Comment on attachment 275069 [details] [diff] [review]
General patch

Checked-in, as per comment #9.
Attachment #275069 - Flags: approval1.9?
Comment on attachment 275296 [details] [diff] [review]
Patch

Checked-in, as per comment #9.
Attachment #275296 - Flags: approval1.9?
Comment on attachment 275296 [details] [diff] [review]
Patch

Reset approval flag to + as it was already checked in.
Attachment #275296 - Flags: approval1.9? → approval1.9+
Comment on attachment 275069 [details] [diff] [review]
General patch

Reset approval flag to + as it was already checked in.
Attachment #275069 - Flags: approval1.9? → approval1.9+
This landed, but there is some question about whether it is fixed (see comments 10 and 11). We want to get it off the list
for "isn't closed but has approved patch for FF3", so I'm closing this and I've
created the followup bug 433050. 
Status: ASSIGNED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → INCOMPLETE
Change of plans...reopening, closing bug 433050, and adding whiteboard flag [not needed for 1.9] as suggested in irc. Sorry about the confusion.
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Whiteboard: [not needed for 1.9]
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.