Summary: | SVGTextElement.getNumberOfChars is broken for altGlyph (affects Acid3 test 79) | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||||
Component: | SVG | Assignee: | Maciej Stachowiak <mjs> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | zimmermann | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 17064 | ||||||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2008-01-29 00:34:48 PST
Created attachment 18759 [details]
reduced test case
TOT returns 32 for this, Safari 3 returns 0. I'm not sure why this should be 33, I'd have to stare at it more closely.
Sorry about the test being ridiculously long. BTW that 33 vs 32 might well be a mistake in the test; it looks like the last string literal in the function-serialised SVG fragment should have an 'A' at the end (i.e., it should be '\uD800\uDC85A' instead of '\uD800\uDC85'), since the rest of the test assumes that there is an A character after the astral character. Created attachment 19031 [details]
better test case
Actually, this just seems completely broken. Created attachment 19032 [details]
Even simpler test case
(In reply to comment #5) > Created an attachment (id=19032) [edit] > Even simpler test case > How did you ever get Acid3 to return sth. different to '0' ??? This test creates a SVG Document, and appends a <text> element, tries to query the metrics... at this point the render tree is not attached(), yet and _any_ call to any SVGTextContentElement will return 0. Greetings, Niko (In reply to comment #6) > How did you ever get Acid3 to return sth. different to '0' ??? > This test creates a SVG Document, and appends a <text> element, tries to query > the metrics... at this point the render tree is not attached(), yet and _any_ > call to any SVGTextContentElement will return 0. In my latest test case the <text> element is actually in the document and rendered. So it seems there are more fundamental problems than just lacking a renderer. (In reply to comment #7) > (In reply to comment #6) > > How did you ever get Acid3 to return sth. different to '0' ??? > > This test creates a SVG Document, and appends a <text> element, tries to query > > the metrics... at this point the render tree is not attached(), yet and _any_ > > call to any SVGTextContentElement will return 0. > > In my latest test case the <text> element is actually in the document and > rendered. So it seems there are more fundamental problems than just lacking a > renderer. I don't actually understand your last testcase. What is it supposed to test? It doesn't work In Opera/Safari/FF or IE6/7. The first testcase is much more valid - and shows the problem is because of missing <altGlyph> support - if you remove the <altGlyph> element the test works. Greetings, Niko Created attachment 19072 [details]
Even simpler test case
> The first testcase is much more valid - and shows the problem is because of
> missing <altGlyph> support - if you remove the <altGlyph> element the test
> works.
Yeah, sorry, I uploaded the wrong test case. I've fixed that. In my final test case, you'll see that getNumberOfChars just doesn't work at all in this case. Maybe the first layout has not yet been done?
>
> Yeah, sorry, I uploaded the wrong test case. I've fixed that. In my final
> test case, you'll see that getNumberOfChars just doesn't work at all in this
> case. Maybe the first layout has not yet been done?
>
That's my guess. I just updated to ToT, and found out the testcases are working - what fixed it?!
Greetings,
Niko
Acid3 was actually buggy on this test (note that it has moved from test 69 to test 79). I got Hixie to fix it so that the test is rendered now (before we had no rendertree because he just did createDocument without putting it anywhere). We now fail this because we report 33 characters instead of 34. Created attachment 19458 [details]
Add a copy of Acid3 test 79
LayoutTests/svg/custom/acid3-test-79.html | 13 +
LayoutTests/svg/custom/resources/acid3-test-77.js | 2 -
LayoutTests/svg/custom/resources/acid3-test-79.js | 283 +++++++++++++++++++++
3 files changed, 296 insertions(+), 2 deletions(-)
I figured out that we get the wrong answer for getNumberOfChars because altGlyph is not implemented. I did a very basic implementation which renders it as a tspan (ignoring the requested glyph substitution). This is enough to get the number of characters right, but more complete support will be needed to get the correct layout that the rest of the test requires. I filed bug 18031 for these subsequent failures. Created attachment 19992 [details]
patch v1
Comment on attachment 19992 [details]
patch v1
Looks great! Congrats on your first SVG patch. And yes... the "adding a tag" process is way way too manual at this point.
|