Bug 18046

Summary: SVGTextElement.getStartPositionOfChar does not correctly account for multichar glyphs
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: SVGAssignee: Maciej Stachowiak <mjs>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 18031    
Attachments:
Description Flags
handle multichar glyph metrics and rendering correctly eric: review+

Maciej Stachowiak
Reported 2008-03-24 13:38:01 PDT
SVGTextElement.getStartPositionOfChar counts the advance of each character in multichar glyphs individually. Instead, it should apply the advance of the whole glyph only after the last char of the multichar glyph, because SVG requires that characters all rendered as a unit are considered to have the same start position. This causes many problems on Acid3 test 79.
Attachments
handle multichar glyph metrics and rendering correctly (53.34 KB, patch)
2008-03-24 14:21 PDT, Maciej Stachowiak 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1 2008-03-24 14:21:43 PDT
Created attachment 20013 [details] handle multichar glyph metrics and rendering correctly
Eric Seidel (no email)
Comment 2 2008-03-24 14:45:32 PDT
Comment on attachment 20013 [details] handle multichar glyph metrics and rendering correctly Looks like you added a space before svgTextRunForInlineTextBox by accident. LayoutTests/platform/mac/svg/text/multichar-glyph-expected.txt is displaying an XML error (when it really probably should dump as text, no?) It's possible to write SVG tests which use the shouldBe() stuff, and are js files, but it's not possible to have a custom SVG template. Your copy/paste of shouldBe is less than ideal, but also OK.
Eric Seidel (no email)
Comment 3 2008-03-24 16:57:51 PDT
Comment on attachment 20013 [details] handle multichar glyph metrics and rendering correctly Aside from my two corrections above, the patch looks sane. extraCharsAvailable could be part of TextRun, but you had performance concerns. I think it's OK to not have it part of TextRun (at least for now).
Maciej Stachowiak
Comment 4 2008-03-24 17:36:17 PDT
Thanks. I fixed the two issues above before landing.
Note You need to log in before you can comment on or make changes to this bug.