Title: [138316] trunk
- Revision
- 138316
- Author
- schen...@chromium.org
- Date
- 2012-12-20 16:13:14 -0800 (Thu, 20 Dec 2012)
Log Message
SVG: <altglpyh> for a surrogate pair character in a ligature fails
https://bugs.webkit.org/show_bug.cgi?id=102969
Reviewed by Dirk Schulze.
Source/WebCore:
There are two issues with SVG <altglyph> tags applied to surrogate
fonts, particularly when mixed with non-standard forms (arabic,
vertical, etc.).
First, there is an assertion that is invalid when an alt glyph is
substituted for the surrogate, because the text chunk that is consumed
by an alt glyph is the entire run, whereas we assert that a
surrogate's chunk is length 2 regardless. That assertion has been
removed.
Second, when an arabic character or some other characters requiring a
special form appears before the surrogate pair character inside the alt
glyph tag, we reject the alt glyph because it is not compatible with the form.
However, when we process the next character - the surrogate pair - we
do accept the alt glyph. This breaks all the indexes because we have
already consumed part of the run that is now considered the alt glyph.
Chaos ensues. This patch forces us to always accept alt glyph
characters (assuming we have some glyph to draw). This better matches
the intent of the spec - if someone specifies an alt glyph they are
explicitly stating which glyph they want used. We should not argue
with the content author.
Tests: svg/text/alt-glyph-for-surrogate-pair-expected.svg
svg/text/alt-glyph-for-surrogate-pair.svg
* rendering/svg/SVGTextLayoutEngine.cpp:
(WebCore::SVGTextLayoutEngine::layoutTextOnLineOrPath): Fix some poor code.
* rendering/svg/SVGTextMetricsBuilder.cpp:
(WebCore::SVGTextMetricsBuilder::advanceSimpleText): Remove an assert that is not always valid.
* svg/SVGFontData.cpp:
(WebCore::SVGFontData::applySVGGlyphSelection): Always return an altGlyph when found. Do not check it compatibility.
LayoutTests:
Test and reference result for altGlyph elements applied to a surrogate pair
following an arabic form. Note that these tests are html, but for the test to fail
without the patch they must be parsed as svg (xml).
* svg/text/alt-glyph-for-surrogate-pair-expected.svg: Added.
* svg/text/alt-glyph-for-surrogate-pair.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (138315 => 138316)
--- trunk/LayoutTests/ChangeLog 2012-12-21 00:09:20 UTC (rev 138315)
+++ trunk/LayoutTests/ChangeLog 2012-12-21 00:13:14 UTC (rev 138316)
@@ -1,3 +1,17 @@
+2012-12-20 Stephen Chenney <schen...@chromium.org>
+
+ SVG: <altglpyh> for a surrogate pair character in a ligature fails
+ https://bugs.webkit.org/show_bug.cgi?id=102969
+
+ Reviewed by Dirk Schulze.
+
+ Test and reference result for altGlyph elements applied to a surrogate pair
+ following an arabic form. Note that these tests are html, but for the test to fail
+ without the patch they must be parsed as svg (xml).
+
+ * svg/text/alt-glyph-for-surrogate-pair-expected.svg: Added.
+ * svg/text/alt-glyph-for-surrogate-pair.svg: Added.
+
2012-12-20 Adam Klein <ad...@chromium.org>
Properly process <template> end tags when in TemplateContentsMode
Added: trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair-expected.svg (0 => 138316)
--- trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair-expected.svg (rev 0)
+++ trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair-expected.svg 2012-12-21 00:13:14 UTC (rev 138316)
@@ -0,0 +1,10 @@
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <svg width="200" height="100" xmlns="http://www.w3.org/2000/svg">
+ <font>
+ <font-face font-family="xyzzy" units-per-em="10"/>
+ <glyph id="sekrit" horiz-adv-x="14" d="m0,0 l10,0 l0,10 l-10,0 z"/>
+ </font>
+ <rect fill="green" x="25" y="9" width="16" height="16"/>
+ <text x="47" y="25" font-family="xyzzy">PASSED</text>
+ </svg>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair.svg (0 => 138316)
--- trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair.svg (rev 0)
+++ trunk/LayoutTests/svg/text/alt-glyph-for-surrogate-pair.svg 2012-12-21 00:13:14 UTC (rev 138316)
@@ -0,0 +1,9 @@
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <svg width="200" height="100" xmlns="http://www.w3.org/2000/svg">
+ <font>
+ <font-face font-family="xyzzy" units-per-em="10"/>
+ <glyph id="sekrit" horiz-adv-x="14" d="m0,0 l10,0 l0,10 l-10,0 z"/>
+ </font>
+ <text x="25" y="25" font-family="xyzzy" stroke-width="10"><altGlyph fill="green" xlink:href=""
+ </svg>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (138315 => 138316)
--- trunk/Source/WebCore/ChangeLog 2012-12-21 00:09:20 UTC (rev 138315)
+++ trunk/Source/WebCore/ChangeLog 2012-12-21 00:13:14 UTC (rev 138316)
@@ -1,3 +1,42 @@
+2012-12-20 Stephen Chenney <schen...@chromium.org>
+
+ SVG: <altglpyh> for a surrogate pair character in a ligature fails
+ https://bugs.webkit.org/show_bug.cgi?id=102969
+
+ Reviewed by Dirk Schulze.
+
+ There are two issues with SVG <altglyph> tags applied to surrogate
+ fonts, particularly when mixed with non-standard forms (arabic,
+ vertical, etc.).
+
+ First, there is an assertion that is invalid when an alt glyph is
+ substituted for the surrogate, because the text chunk that is consumed
+ by an alt glyph is the entire run, whereas we assert that a
+ surrogate's chunk is length 2 regardless. That assertion has been
+ removed.
+
+ Second, when an arabic character or some other characters requiring a
+ special form appears before the surrogate pair character inside the alt
+ glyph tag, we reject the alt glyph because it is not compatible with the form.
+ However, when we process the next character - the surrogate pair - we
+ do accept the alt glyph. This breaks all the indexes because we have
+ already consumed part of the run that is now considered the alt glyph.
+ Chaos ensues. This patch forces us to always accept alt glyph
+ characters (assuming we have some glyph to draw). This better matches
+ the intent of the spec - if someone specifies an alt glyph they are
+ explicitly stating which glyph they want used. We should not argue
+ with the content author.
+
+ Tests: svg/text/alt-glyph-for-surrogate-pair-expected.svg
+ svg/text/alt-glyph-for-surrogate-pair.svg
+
+ * rendering/svg/SVGTextLayoutEngine.cpp:
+ (WebCore::SVGTextLayoutEngine::layoutTextOnLineOrPath): Fix some poor code.
+ * rendering/svg/SVGTextMetricsBuilder.cpp:
+ (WebCore::SVGTextMetricsBuilder::advanceSimpleText): Remove an assert that is not always valid.
+ * svg/SVGFontData.cpp:
+ (WebCore::SVGFontData::applySVGGlyphSelection): Always return an altGlyph when found. Do not check it compatibility.
+
2012-12-20 Adam Klein <ad...@chromium.org>
Properly process <template> end tags when in TemplateContentsMode
Modified: trunk/Source/WebCore/rendering/svg/SVGTextLayoutEngine.cpp (138315 => 138316)
--- trunk/Source/WebCore/rendering/svg/SVGTextLayoutEngine.cpp 2012-12-21 00:09:20 UTC (rev 138315)
+++ trunk/Source/WebCore/rendering/svg/SVGTextLayoutEngine.cpp 2012-12-21 00:13:14 UTC (rev 138316)
@@ -578,21 +578,10 @@
y += m_dy;
}
- // Determine wheter we have to start a new fragment.
- bool shouldStartNewFragment = false;
+ // Determine whether we have to start a new fragment.
+ bool shouldStartNewFragment = m_dx || m_dy || m_isVerticalText || m_inPathLayout || angle || angle != lastAngle
+ || orientationAngle || kerning || applySpacingToNextCharacter || definesTextLength;
- if (m_dx || m_dy)
- shouldStartNewFragment = true;
-
- if (!shouldStartNewFragment && (m_isVerticalText || m_inPathLayout))
- shouldStartNewFragment = true;
-
- if (!shouldStartNewFragment && (angle || angle != lastAngle || orientationAngle))
- shouldStartNewFragment = true;
-
- if (!shouldStartNewFragment && (kerning || applySpacingToNextCharacter || definesTextLength))
- shouldStartNewFragment = true;
-
// If we already started a fragment, close it now.
if (didStartTextFragment && shouldStartNewFragment) {
applySpacingToNextCharacter = false;
Modified: trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp (138315 => 138316)
--- trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp 2012-12-21 00:09:20 UTC (rev 138315)
+++ trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp 2012-12-21 00:13:14 UTC (rev 138316)
@@ -65,9 +65,6 @@
return;
}
- if (currentCharacterStartsSurrogatePair())
- ASSERT(metricsLength == 2);
-
float currentWidth = m_simpleWidthIterator->runWidthSoFar() - m_totalWidth;
m_totalWidth = m_simpleWidthIterator->runWidthSoFar();
Modified: trunk/Source/WebCore/svg/SVGFontData.cpp (138315 => 138316)
--- trunk/Source/WebCore/svg/SVGFontData.cpp 2012-12-21 00:09:20 UTC (rev 138315)
+++ trunk/Source/WebCore/svg/SVGFontData.cpp 2012-12-21 00:13:14 UTC (rev 138316)
@@ -189,6 +189,16 @@
size_t glyphsSize = glyphs.size();
for (size_t i = 0; i < glyphsSize; ++i)
glyphs[i].unicodeStringLength = run.length();
+
+ // Do not check alt glyphs for compatibility. Just return the first one.
+ // Later code will fail if we do not do this and the glyph is incompatible.
+ if (glyphsSize) {
+ SVGGlyph& svgGlyph = glyphs[0];
+ iterator.setLastGlyphName(svgGlyph.glyphName);
+ glyphData.glyph = svgGlyph.tableEntry;
+ advanceLength = svgGlyph.unicodeStringLength;
+ return true;
+ }
} else
associatedFontElement->collectGlyphsForString(remainingTextInRun, glyphs);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
