Title: [130999] trunk
- Revision
- 130999
- Author
- [email protected]
- Date
- 2012-10-10 19:40:44 -0700 (Wed, 10 Oct 2012)
Log Message
SVGTextRunRenderingContext changes font data in the glyph page, but it shouldn't
https://bugs.webkit.org/show_bug.cgi?id=98755
Reviewed by Eric Seidel.
Source/WebCore:
The code in SVGTextRunRenderingContext::glyphDataForCharacter, when it
encounters an <altglyph> tag, immediately replaces the font data for a
glyph with font data for the primary font, presumably to meet the SVG
spec requirement: "If the references to alternate glyphs do not result
in successful identification of alternate glyphs to use, then the
character(s) that are inside of the ‘altGlyph’ element are rendered as
if the ‘altGlyph’ element were a ‘tspan’ element instead."
If the alt glyph is not then found we are in the case from the spec
and indeed we should use the primary font. However, we end up replacing the GlyphPage
entry for the character with primary font data, which we should not do
because the glyph page might be used in some place that does not have
the alt glyph tag.
Furthermore, this causes object lifetime problems for font data, because
in cases where the font data that is replaced is for the system fallback
font the GlyphPage will live forever with no knowldege that it contains
font data pointers into font data other that the system fallback. The
replaced font data may be deleted while the pointer lives on in the
system fallback page.
The fix is simply not to replace the font data in the page.
Test: svg/text/alt-glpyh-on-fallback-font-crash.html
* rendering/svg/SVGTextRunRenderingContext.cpp:
(WebCore::SVGTextRunRenderingContext::glyphDataForCharacter): Keep track of the original font data and put it back
in the glyph page when the method has finished.
LayoutTests:
New test case that includes an alt-glyph that comes from the system
fallback font (because the alt-glyph doesn't reference anything). This
test crashes on Chromium linux without the patch, and may crash on
other platforms too.
* svg/text/alt-glpyh-on-fallback-font-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (130998 => 130999)
--- trunk/LayoutTests/ChangeLog 2012-10-11 02:33:49 UTC (rev 130998)
+++ trunk/LayoutTests/ChangeLog 2012-10-11 02:40:44 UTC (rev 130999)
@@ -1,3 +1,17 @@
+2012-10-10 Stephen Chenney <[email protected]>
+
+ SVGTextRunRenderingContext changes font data in the glyph page, but it shouldn't
+ https://bugs.webkit.org/show_bug.cgi?id=98755
+
+ Reviewed by Eric Seidel.
+
+ New test case that includes an alt-glyph that comes from the system
+ fallback font (because the alt-glyph doesn't reference anything). This
+ test crashes on Chromium linux without the patch, and may crash on
+ other platforms too.
+
+ * svg/text/alt-glpyh-on-fallback-font-crash.html: Added.
+
2012-10-10 Tab Atkins <[email protected]>
column-count: 0 should not prevent margin-collapse through
Added: trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt (0 => 130999)
--- trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt 2012-10-11 02:40:44 UTC (rev 130999)
@@ -0,0 +1 @@
+A A: This test passes if you see two As at the start of this message.
Added: trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg (0 => 130999)
--- trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg (rev 0)
+++ trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg 2012-10-11 02:40:44 UTC (rev 130999)
@@ -0,0 +1,23 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+ <g>
+ <defs>
+ <font>
+ <font-face font-family="HappySad">
+ </font-face>
+ </font>
+ </defs>
+ <g font-family="HappySad" style="-webkit-writing-mode: vertical-rl; ">
+ <text>
+ <altGlyph>A</altGlyph>
+ A: This test passes if you see two As at the start of this message.
+ </text>
+ </g>
+ </g>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ var docElement = document.body ? document.body : document.documentElement;
+ if (docElement) docElement.offsetTop;
+ </script>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (130998 => 130999)
--- trunk/Source/WebCore/ChangeLog 2012-10-11 02:33:49 UTC (rev 130998)
+++ trunk/Source/WebCore/ChangeLog 2012-10-11 02:40:44 UTC (rev 130999)
@@ -1,3 +1,39 @@
+2012-10-10 Stephen Chenney <[email protected]>
+
+ SVGTextRunRenderingContext changes font data in the glyph page, but it shouldn't
+ https://bugs.webkit.org/show_bug.cgi?id=98755
+
+ Reviewed by Eric Seidel.
+
+ The code in SVGTextRunRenderingContext::glyphDataForCharacter, when it
+ encounters an <altglyph> tag, immediately replaces the font data for a
+ glyph with font data for the primary font, presumably to meet the SVG
+ spec requirement: "If the references to alternate glyphs do not result
+ in successful identification of alternate glyphs to use, then the
+ character(s) that are inside of the ‘altGlyph’ element are rendered as
+ if the ‘altGlyph’ element were a ‘tspan’ element instead."
+
+ If the alt glyph is not then found we are in the case from the spec
+ and indeed we should use the primary font. However, we end up replacing the GlyphPage
+ entry for the character with primary font data, which we should not do
+ because the glyph page might be used in some place that does not have
+ the alt glyph tag.
+
+ Furthermore, this causes object lifetime problems for font data, because
+ in cases where the font data that is replaced is for the system fallback
+ font the GlyphPage will live forever with no knowldege that it contains
+ font data pointers into font data other that the system fallback. The
+ replaced font data may be deleted while the pointer lives on in the
+ system fallback page.
+
+ The fix is simply not to replace the font data in the page.
+
+ Test: svg/text/alt-glpyh-on-fallback-font-crash.html
+
+ * rendering/svg/SVGTextRunRenderingContext.cpp:
+ (WebCore::SVGTextRunRenderingContext::glyphDataForCharacter): Keep track of the original font data and put it back
+ in the glyph page when the method has finished.
+
2012-10-10 Tab Atkins <[email protected]>
column-count: 0 should not prevent margin-collapse through
Modified: trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp (130998 => 130999)
--- trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp 2012-10-11 02:33:49 UTC (rev 130998)
+++ trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp 2012-10-11 02:40:44 UTC (rev 130999)
@@ -188,6 +188,7 @@
}
// Characters enclosed by an <altGlyph> element, may not be registered in the GlyphPage.
+ const SimpleFontData* originalFontData = glyphData.fontData;
if (glyphData.fontData && !glyphData.fontData->isSVGFont()) {
if (TextRun::RenderingContext* renderingContext = run.renderingContext()) {
RenderObject* renderObject = static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
@@ -239,7 +240,7 @@
// Restore original state of the SVG Font glyph table and the current font fallback list,
// to assure the next lookup of the same glyph won't immediately return the fallback glyph.
- page->setGlyphDataForCharacter(character, glyphData.glyph, fontData);
+ page->setGlyphDataForCharacter(character, glyphData.glyph, originalFontData);
fontList->setGlyphPageZero(originalGlyphPageZero);
fontList->setGlyphPages(originalGlyphPages);
ASSERT(fallbackGlyphData.fontData);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
