Bug 17062 – SVGTextElement.getNumberOfChars is broken for altGlyph (affects Acid3 test 79)

• Component: SVG
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Test 69: expected 33, got: 0 - SVGSVGTextElement.getNumberOfChars() incorrect

Test 78: expected 3, got: 0 - getNumberOfChars returned incorrect string length.
also reports a similar failure.

The ridiculously long test case:

    function () {
      // test 69: a giant test for <svg:font>, from Cameron McCormack
      // This tests various features of SVG fonts from SVG 1.1.  It consists of
      // a <text> element with 33 characters, styled using an SVG font that has
      // different advance values for each glyph.  The script uses
      // SVGTextElementContent.getStartPositionOfChar() to determine where the
      // glyph corresponding to each character was placed, and thus to work out
      // whether the SVG font was used correctly.
      //
      // The font uses 100 units per em, and the text is set in 100px.  Since
      // font-size gives the size of the em box
      // (http://www.w3.org/TR/SVG11/text.html#DOMInterfaces), the scale of the
      // coordinate system for the glyphs is the same as the SVG document.
      //
      // The expectedAdvances array holds the expected advance value for each
      // character, and expectedKerning holds the (negative) kerning for each
      // character.  getPositionOfChar() returns the actual x coordinate for the
      // glyph, corresponding to the given character, and if multiple characters
      // correspond to the same glyph, the same position value is returned for
      // each of those characters.
      //
      // Here are the reasonings for the advance/kerning values.  Note that for
      // a given character at index i, the expected position is
      // sum(expectedAdvances[0:i-1] + expectedKerning[0:i-1]).
      //
      // char     advance  kerning  reasoning
      // -------  -------  -------  --------------------------------------------------
      // A        10000    0        Normal character mapping to a single glyph.
      // B        0        0        First character of a two character glyph, so the
      //                            current position isn't advanced until the second
      //                            character.
      // C        200      0        Second character of a two character glyph, so now
      //                            the position is advanced.
      // B        300      0        Although there is a glyph for "BC" in the font,
      //                            it appears after the glyph for "B", so the single
      //                            character glyph for "B" should be chosen instead.
      // D        1100     0        Normal character mapping to a single glyph.
      // A        10000    200      Kerning of -200 is specified in the font between
      //                            the "A" and "EE" glyphs.
      // E        0        0        The first character of a two character glyph "EE".
      // E        1300     0        The second character of a two character glyph.
      // U        0        0        This is a glyph for the six characters "U+0046",
      //                            which happen to look like a valid unicode range.
      //                            This tests that the <glyph unicode=""> in the
      //                            font matches exact strings rather than a range,
      //                            as used in the kerning elements.
      // +        0        0        Second character of six character glyph.
      // 0        0        0        Third character of six character glyph.
      // 0        0        0        Fourth character of six character glyph.
      // 4        0        0        Fifth character of six character glyph.
      // 6        1700     0        Sixth character of six character glyph.
      // U        0        0        The same six character glyph that looks like a
      //                            Unicode range.  One of the kerning elements has
      //                            u1="U+0046" u2="U+0046", which shouldn't match
      //                            this, because those attributes are interpreted
      //                            as Unicode ranges if they are, and normal
      //                            strings otherwise.  Thus there should be no
      //                            kerning between these two glyphs.
      // G        2300     200      Kerning is between this character and the next
      //                            "G", since there is an <hkern> element that
      //                            uses a Unicode range on its u1="" attribute
      //                            and a glyph name on its g2="" attribute which
      //                            both match "G".
      // G        2300     0        Normal character with kerning before it.
      // H        3100     0        A glyph with graphical content describing the
      //                            glyph, rather than a d="" attribute.
      // I        4300     0        Glyphs are checked in document order for one
      //                            that matches, but the first glyph with
      //                            unicode="I" also has lang="zh", which disqualifies
      //                            it.  Thus the second glyph with unicode="I"
      //                            is chosen.
      // I        4100     0        Since this I has xml:lang="zh" on it in the text,
      //                            the first glyph with lang="zh" matches.
      // J        4700     -4700    A normal glyph with kerning between the "J" and the
      //                            next glyph "A" equal to the advance of the "J"
      //                            glyph, so the position should stay the same.
      // A        10000    0        Normal glyph with kerning before it.
      // K        5900     0        The first glyph with unicode="K" does not match,
      //                            since it has orientation="v", so the second
      //                            glyph with unicode="K" is chosen.
      // <spc>    6100     0        The space character should select the glyph with
      //                            unicode=" ", despite it having a misleading
      //                            glyph-name="L".
      // L        6700     0        The "L" character should select the glyph with
      //                            unicode=" ", despite it having a misleading
      //                            glyph-name="spacev".
      // A        2900     0        An <altGlyph> element is used to select the
      //                            glyph for U+10085 instead of the one for "A".
      // U+10085  2900     0        Tests glyph selection with a non-plane-0
      //                            character.
      // A        10000    0        A final normal character.
      //
      // In addition, the script tests the value returned by
      // SVGTextContentElement.getNumberOfChars(), which in this case should be 33.
      // If it returned 34, then it incorrectly counted UTF-16 codepoints or
      // something.
      //
      // See http://www.w3.org/TR/SVG11/fonts.html for a description of the glyph
      // matching rules, and http://www.w3.org/TR/SVG11/text.html#DOMInterfaces
      // for a description of getStartPositionOfChar() and getNumberOfChars().
      //
      // Note also that the test uses DOMImplementation.createDocument() to create
      // the SVG document.  This seems to cause browsers trouble for the SVG DOM
      // interfaces, since the document isn't being "rendered" as it might be
      // if it were in an <iframe>.  Changing the test to use an <iframe> will
      // at least let you see the main part of the test running.

      var NS = { 
        svg: 'http://www.w3.org/2000/svg',
        xml: 'http://www.w3.org/1998/XML/namespace',
        xlink: 'http://www.w3.org/1999/xlink'
      };

      var doc = document.implementation.createDocument(NS.svg, 'svg', null);
      var e = function (n, as, cs) {
        var elt = doc.createElementNS(NS.svg, n);
        if (as) {
          for (var an in as) {
            var idx = an.indexOf(':');
            var ns = null;
            if (idx != -1)
              ns = NS[an.substring(0, idx)];
            elt.setAttributeNS(ns, an, as[an]);
          }
        }
        if (cs) {
          for (var i in cs) {
            var c = cs[i];
            elt.appendChild(typeof c == 'string' ? doc.createTextNode(c) : c);
          }
        }
        return elt;
      }
      doc.documentElement.appendChild(e('font', { 'horiz-adv-x': '10000'}, [e('font-face', { 'font-family': 'HCl', 'units-per-em': '100', 'ascent': '1000', 'descent': '500'}), e('missing-glyph', null, [e('path', { 'd': 'M100,0 h800 v-100 h-800 z'})]), e('glyph', { 'unicode': 'A', 'd': 'M100,0 h100 v-100 h-100 z'}), e('glyph', { 'unicode': 'BC', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '200'}), e('glyph', { 'unicode': 'B', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '300'}), e('glyph', { 'unicode': 'C', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '500'}), e('glyph', { 'unicode': 'BD', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '700'}), e('glyph', { 'unicode': 'D', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '1100'}), e('glyph', { 'unicode': 'EE', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '1300'}), e('glyph', { 'unicode': 'U+0046', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '1700'}), e('glyph', { 'unicode': 'F', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '1900'}), e('glyph', { 'unicode': 'G', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '2300', 'glyph-name': 'gee'}), e('glyph', { 'unicode': '\uD800\uDC85', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '2900', 'id': 'astral'}), e('glyph', { 'unicode': 'H', 'horiz-adv-x': '3100'}, [e('path', { 'd': 'M100,0 h100 v-100 h-100 z'})]), e('glyph', { 'unicode': 'I', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '4100', 'lang': 'zh'}), e('glyph', { 'unicode': 'I', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '4300'}), e('glyph', { 'unicode': 'J', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '4700'}), e('glyph', { 'unicode': 'K', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '5300', 'orientation': 'v'}), e('glyph', { 'unicode': 'K', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '5900'}), e('glyph', { 'unicode': ' ', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '6100', 'glyph-name': 'L'}), e('glyph', { 'unicode': 'L', 'd': 'M100,0 h100 v-100 h-100 z', 'horiz-adv-x': '6700', 'glyph-name': 'space'}), e('hkern', { 'u1': 'A', 'u2': 'EE', 'k': '-200'}), e('hkern', { 'u1': 'U+0046', 'u2': 'U+0046', 'k': '-200'}), e('hkern', { 'u1': 'U+0047-0047', 'g2': 'gee', 'k': '-200'}), e('hkern', { 'u1': 'J', 'u2': 'A', 'k': '4700'})]));
      doc.documentElement.appendChild(e('text', { 'y': '100', 'font-family': 'HCl', 'font-size': '100px', 'letter-spacing': '0px', 'word-spacing': '0px'}, ['ABCBDAEEU+0046U+0046GGHI', e('tspan', { 'xml:lang': 'zh'}, ['I']), 'JAK L', e('altGlyph', { 'xlink:href': '#astral'}, ['A']), '\uD800\uDC85']));

      var t = doc.documentElement.lastChild;

      var characterDescriptions = [
        "a normal character",
        "the first character of a two-character glyph",
        "the second character of a two-character glyph",
        "a normal character, which shouldn't be the first character of a two-character glyph",
        "a normal character, which shouldn't be the second character of a two-character glyph",
        "a normal character, which has some kerning after it",
        "the first character of a two-character glyph, which has some kerning before it",
        "the second character of a two-character glyph, which has some kerning before it",
        "the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not",
        "the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not",
        "a normal character, which has some kerning after it that is specified by glyph name",
        "a normal character, which has some kerning before it that is specified by glyph name",
        "a normal character, whose glyph is given by child graphical content of the <glyph> element",
        "a normal character, whose glyph should not match the one with a lang=\"\" attribute on it",
        "a normal character, whose glyph should match the one with a lang=\"\" attribute on it",
        "a normal character, which has some kerning after it that is equal to the advance of the character",
        "a normal character, which has some kerning before it that is equal to the advance of the previous character",
        "a normal character, whose glyph should not match the one with an orientation=\"v\" attribute on it",
        "a space character, which has a misleading glyph-name=\"\" attribute",
        "a normal character, which has a misleading glyph-name=\"\" attribute",
        "a normal character, whose glyph is chosen to be another by using <altGlyph>",
        "a character not in Plane 0",
        "a normal character",
      ];

      var expectedAdvances = [
        10000,       // A
        0,           // BC [0]
        200,         // BC [1]
        300,         // B
        1100,        // D
        10000,       // A
        0,           // EE [0]
        1300,        // EE [1]
        0,           // U+0046 [0]
        0,           // U+0046 [1]
        0,           // U+0046 [2]
        0,           // U+0046 [3]
        0,           // U+0046 [4]
        1700,        // U+0046 [5]
        0,           // U+0046 [0]
        0,           // U+0046 [1]
        0,           // U+0046 [2]
        0,           // U+0046 [3]
        0,           // U+0046 [4]
        1700,        // U+0046 [5]
        2300,        // G
        2300,        // G
        3100,        // H
        4300,        // I
        4100,        // I (zh)
        4700,        // J
        10000,       // A
        5900,        // K
        6100,        // <space>
        6700,        // L
        2900,        // A (using &#x10085; altGlyph)
        2900,        // &#x10085;
        10000,       // A
      ];

      var expectedKerning = [
        0,           // A
        0,           // BC [0]
        0,           // BC [1]
        0,           // B
        0,           // D
        200,         // A
        0,           // EE [0]
        0,           // EE [1]
        0,           // U+0046 [0]
        0,           // U+0046 [1]
        0,           // U+0046 [2]
        0,           // U+0046 [3]
        0,           // U+0046 [4]
        0,           // U+0046 [5]
        0,           // U+0046 [0]
        0,           // U+0046 [1]
        0,           // U+0046 [2]
        0,           // U+0046 [3]
        0,           // U+0046 [4]
        0,           // U+0046 [5]
        200,         // G
        0,           // G
        0,           // H
        0,           // I
        0,           // I (zh)
        -4700,       // J
        0,           // A
        0,           // K
        0,           // <space>
        0,           // L
        0,           // A (using &#x10085; altGlyph)
        0,           // &#x10085;
        0,           // A
      ];

      var numberOfChars = t.getNumberOfChars();
      if (numberOfChars == expectedAdvances.length + 1)
        fail('SVGSVGTextElement.getNumberOfChars() counts UTF-16 surrogates as separate characters');
      assertEquals(numberOfChars, expectedAdvances.length, 'SVGSVGTextElement.getNumberOfChars() incorrect');

      var expectedPositions = [0];
      for (var i = 0; i < expectedAdvances.length; i++)
        expectedPositions.push(expectedPositions[i] + expectedAdvances[i] + expectedKerning[i]);

      var actualPositions = [];
      for (var i = 0; i < numberOfChars; i++)
        actualPositions.push(t.getStartPositionOfChar(i).x);
      actualPositions.push(t.getEndPositionOfChar(numberOfChars - 1).x);

      for (var i = 0; i < expectedPositions.length; i++) {
        if (expectedPositions[i] != actualPositions[i]) {
          var s = 'Position of character ' + i + ' was ' + actualPositions[i] + ', but expecting ' + expectedPositions[i] + ', which was ';
          if (i == 0) {
            s += 'before ' + characterDescriptions[0] + '.';
          } else if (i == expectedPositions.length - 1) {
            s += 'after ' + characterDescriptions[characterDescriptions.length - 1] + '.';
          } else {
            s += 'between:\n\n    ' + characterDescriptions[i - 1] + ',\n\nand:\n\n    ' + characterDescriptions[i] + '.';
          }
          fail(s);
        }
      }
      return 5;
    },

[Eric Seidel (no email)]

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.

[Cameron McCormack (:heycam)]

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.

[Eric Seidel (no email)]

Created attachment 19031 [details]
better test case

[Eric Seidel (no email)]

Actually, this just seems completely broken.

[Eric Seidel (no email)]

Created attachment 19032 [details]
Even simpler test case

[Nikolas Zimmermann]

(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

[Eric Seidel (no email)]

(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.

[Nikolas Zimmermann]

(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

[Eric Seidel (no email)]

Created attachment 19072 [details]
Even simpler test case

[Eric Seidel (no email)]

> 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?

[Nikolas Zimmermann]

> 
> 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

[Dave Hyatt]

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.

[Eric Seidel (no email)]

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(-)

[Maciej Stachowiak]

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.

[Maciej Stachowiak]

Created attachment 19992 [details]
patch v1

[Eric Seidel (no email)]

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.