Bug 104768 – Uninitialized text decoration color is propagated to ruby text.

• Component: Layout and Rendering
• Status: RESOLVED WONTFIX

[Yuki Sekiguchi]

When I try to apply the following patch to QtWebKit of Qt 4.8, the problem is occurred.

> text-decoration: element should not inherit text-decoration property
> http://trac.webkit.org/changeset/108690

I use the following html file.

> <ruby style="text-decoration: underline">aaa<rt>aa</rt></ruby>

The ruby text "aa" have black underline when I tried.

The propagation of text-decoration is executed at InlineTextBox::paintDecoration()

>    // Get the text decoration colors.
>    Color underline, overline, linethrough;
>    renderer()->getTextDecorationColors(deco, underline, overline, linethrough, true, isFirstLineStyle());
RenderObject::getTextDecorationColors() is...

>    RenderObject* curr = this;
>    RenderStyle* styleToUse = 0;
>    do {
>        styleToUse = curr->style(firstlineStyle);
>        int currDecs = styleToUse->textDecoration();
>        if (currDecs) {
>            if (currDecs & UNDERLINE) {
>                decorations &= ~UNDERLINE;
>                underline = decorationColor(styleToUse);
>            }
>            if (currDecs & OVERLINE) {
>                decorations &= ~OVERLINE;
>                overline = decorationColor(styleToUse);
>            }
>            if (currDecs & LINE_THROUGH) {
>                decorations &= ~LINE_THROUGH;
>                linethrough = decorationColor(styleToUse);
>            }
>        }
>        if (curr->isRubyText())
>            return;
>        curr = curr->parent();
>        if (curr && curr->isAnonymousBlock() && toRenderBlock(curr)->continuation())
>            curr = toRenderBlock(curr)->continuation();
>    } while (curr && decorations && (!quirksMode || !curr->node() ||
>                                     (!curr->node()->hasTagName(aTag) && !curr->node()->hasTagName(fontTag))));

If parent of a text is ruby text, underline, overline and linethrough are uninitialized.

[Yuki Sekiguchi]

Created attachment 178975 [details]
Patch

[Ryosuke Niwa]

Do we have a test case where this problem occurs? If so, we should be adding a new test for this.

[Ryosuke Niwa]

Comment on attachment 178975 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178975&action=review

> Source/WebCore/rendering/InlineTextBox.cpp:978
> +    underline = overline = linethrough = Color::transparent;

The fix doesn't seem right. getTextDecorationColors should be assigning correct values. If not, we need to fix the bug in getTextDecorationColors instead.

[Yuki Sekiguchi]

(In reply to comment #3)
> (From update of attachment 178975 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178975&action=review
> 
> > Source/WebCore/rendering/InlineTextBox.cpp:978
> > +    underline = overline = linethrough = Color::transparent;
> 
> The fix doesn't seem right. getTextDecorationColors should be assigning correct values. If not, we need to fix the bug in getTextDecorationColors instead.

getTextDecorationColors() is used twice.
First is propagating normal text decoration color
Second is overwriting it  by first line color.

Setting transparent at the beginning of getTextDecorationColors() or RubyText "if" statement don't work well because 2nd getTextDecorationColors() don't know what is set by 1st getTextDecorationColors().

I think first line overwrite should be done at getTextDecorationColors().

However, getTextDecorationColors() have another bug, which is bug 104786, and the fix will depend on that.

Therefore, I fix this after fix bug 104786.

[Koji Ishii]

Yuki, can you explain why this bug only occurs on Qt 4.8?

Ryosuke's point is that, if this is about uninitialized local variables, it's very likely to happen on other platforms as well. I don't think all other compilers initialize local variables to zero.

Can you debug on, say, Mac, and see if these variables are initialized automatically? If so, assign random values on initialize in the debugger and then try to reproduce? That'd help reviewers to know that this is really a core bug to be fixed outside of Qt platform code or not.

[Yuki Sekiguchi]

Thank you good advice, Ishii-san.

I recheck Qt code, and I find my understaning is wrong.
However, I think this code need to fix.

The propagation of text-decoration is executed at InlineTextBox::paintDecoration()
>    // Get the text decoration colors.
>    Color underline, overline, linethrough;
>    renderer()->getTextDecorationColors(deco, underline, overline, linethrough, true, isFirstLineStyle());

Color is class, so its constructer is excuted and its m_color becomes transparent and its m_valid becomes false.

RenderObject::getTextDecorationColors() skips color propagation of RenderRuby.
Therefore, text decoration flags of deco variable in InlineTextBox::paintDecoration() remain.

paintDecoration() draws text decoration with invalid transparent Color.

GrapchisContext::setPlatformStrokeColor() sets color to its platform GraphicsContext.

If color is invalid, it does nothing in the Qt imeplementation.
However, it just sets invalid transparent color in CoreGraphics and Skia implementation.

I don't know which implementation is correct.
However, paintDecoration() should not use RGB of invalid Color.
It or getTextDecorationColors() should set valid color to underline, overline and linethrough.

[Yuki Sekiguchi]

Qt is gone.