Bug 24682 – SVG glyph rotation for vertical text is incomplete, and possibly crashy

• Status: RESOLVED
• Resolution: WONTFIX
• Component: SVG

[Eric Seidel (no email)]

SVG glyph rotation for vertical text is incomplete, and possibly crashy

I started down this by reading some static analysis reports about WebCore which showed that this function:

unsigned int findCharUnicodeRange(UChar32 ch)
{
    if (ch >= 0xFFFF)
        return 0;

    unsigned int range;

    //search the first table
    range = gUnicodeSubrangeTable[0][ch >> 12];
  
    if (range < cRangeTableBase)
        // we try to get a specific range 
        return range;

    // otherwise, we have one more table to look at
    range = gUnicodeSubrangeTable[range - cRangeTableBase][(ch & 0x0f00) >> 8];
    if (range < cRangeTableBase)
        return range;
    if (range < cRangeTertiaryTable)
        return gUnicodeSubrangeTable[range - cRangeTableBase][(ch & 0x00f0) >> 4];

    // Yet another table to look at : U+0700 - U+16FF : 128 code point blocks
    return gUnicodeTertiaryRangeTable[(ch - 0x0700) >> 7];
}


could be made to crash, if it's possible for "range" to ever be 144 (Aka cRangeTableBase + 16).  144 is < cRangeTertiaryTable (145) so return gUnicodeSubrangeTable[range - cRangeTableBase][(ch & 0x00f0) >> 4]; will then crash.

Fortunately cRangeTableBase + 16 is not currently found in gUnicodeSubrangeTable so we'll never hit that path.

This got me wondering what this code was for?  Turns out it's only ever called by:

static inline float glyphOrientationToAngle(const SVGRenderStyle* svgStyle, bool isVerticalText, const UChar& character)
{
    switch (isVerticalText ? svgStyle->glyphOrientationVertical() : svgStyle->glyphOrientationHorizontal()) {
    case GO_AUTO:
    {
        // Spec: Fullwidth ideographic and fullwidth Latin text will be set with a glyph-orientation of 0-degrees.
        //       Text which is not fullwidth will be set with a glyph-orientation of 90-degrees.
        unsigned int unicodeRange = findCharUnicodeRange(character);
        if (unicodeRange == cRangeSetLatin || unicodeRange == cRangeArabic)
            return 90.0f;

        return 0.0f;
    }

Which, looking at the spec is probably wrong.  We're only ever rotating latin and arabic text when drawn vertically.  Probably basically all scripts but for a few eastasian scripts want text to rotate 90 degrees when written vertically in the "Default" orientation.

Talking about this with Alexey, we decided that the following code would probably work for ICU platforms:

bool glyphShouldBeRotatedWhenDrawnVertically(UChar ch)
{
  return u_getIntPropertyValue(ch, UCHAR_EAST_ASIAN_WIDTH) != U_EA_FULLWIDTH
}
A simpler version of the current code could be used for non-icu platforms w/o causing any regressions.

For now I'm leaving this.  But it would be "easy" to introduce a crasher to this code w/o trying to.  So we should get rid of this complicated code and replace it with something more correct and easier to read. :)

[Eric Seidel (no email)]

I'm not sure this old bug is still useful.

[Ahmad Saleem]

Unable to find these functions and also too old and as mentioned by reporter after three years that it is not super useful.

Marking this as 'RESOLVED WONTFIX'.

Please reopen if it is still useful.