Closed
Bug 378695
Opened 17 years ago
Closed 17 years ago
SetupClusterBoundaries in gfxPangoFonts.cpp misinterprets attrs from pango_break
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha5
People
(Reporter: karlt, Assigned: karlt)
Details
Attachments
(1 file, 1 obsolete file)
2.33 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
SetupClusterBoundaries in gfxPangoFonts.cpp expects the pango_break argument attrs to have one PangoLogAttr per UTF8 byte but pango_break sets one PangoLogAttr per (unicode) character. I checked the pango_break behaviour in the source code and in its runtime output. pango_default_break in pango-1.16.2/pango/break.c uses g_utf8_next_char to increment the text pointer each iteration but ++ to increment the index into attrs. (Similarly tailor_segment uses g_utf8_strlen.) The attached patch corrects SetupClusterBoundaries. I haven't noticed any side effects from the misinterpretation of the data, at least partially because Bug #332739 is not yet implemented.
Attachment #262718 -
Flags: review?(roc)
Assignee | ||
Comment 1•17 years ago
|
||
Fixes a comment re g_utf8_strlen and makes some code fit within 80 columns.
Attachment #262718 -
Attachment is obsolete: true
Attachment #262725 -
Flags: review?(roc)
Attachment #262718 -
Flags: review?(roc)
Comment 2•17 years ago
|
||
(In reply to comment #1) > Created an attachment (id=262725) [details] > patch v1.1 > > Fixes a comment re g_utf8_strlen and makes some code fit within 80 columns. you can use 120 columns in thebes.
Attachment #262725 -
Flags: review?(roc) → review+
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 262725 [details] [diff] [review] patch v1.1 I don't yet have a CVS account, and roc is traveling so if you are willing and able to checkin this patch, that would be much appreciated, thanks.
Attachment #262725 -
Flags: superreview?(vladimir)
Attachment #262725 -
Flags: superreview?(vladimir)
It doesn't need to be vlad, it could be anyone. You could ask on IRC, someone would probably be able to help. Also, put [checkin needed] on the status whiteboard, some people search for that.
Whiteboard: [checkin needed]
Assignee | ||
Comment 5•17 years ago
|
||
Comment on attachment 262725 [details] [diff] [review] patch v1.1 Thanks for [checkin needed] but don't we need an sr+?
Attachment #262725 -
Flags: superreview?(roc)
Comment 6•17 years ago
|
||
patch for thebes doesn't need the sr.
Assignee | ||
Updated•17 years ago
|
Attachment #262725 -
Flags: superreview?(roc)
Comment 7•17 years ago
|
||
mozilla/gfx/thebes/src/gfxPangoFonts.cpp 1.67
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha5
You need to log in
before you can comment on or make changes to this bug.
Description
•