[Webkit-unassigned] [Bug 88580] RenderObject should not know about first-line element

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88580





--- Comment #2 from Levi Weintraub <leviw at chromium.org>  2012-06-07 16:49:22 PST ---
(From update of attachment 146408)
View in context: https://bugs.webkit.org/attachment.cgi?id=146408&action=review

> Source/WebCore/rendering/InlineBox.h:278
> +    RenderStyle* styleForFirstLine(bool isStyleForFirstLine) const
> +    { 
> +        RenderStyle* style = m_renderer->style();
> +        if (isStyleForFirstLine) {
> +            if (m_renderer->isText())
> +                style = toRenderBoxModelObject(m_renderer->parent())->styleForFirstLine(isStyleForFirstLine);
> +            else
> +                style = boxModelObject()->styleForFirstLine(isStyleForFirstLine);
> +        }
> +        return style;
> +    }

This naming confuses me. If the function is called styleForFirstLine I don't expect to call it with a bool saying whether I actually want the first line.

Also, this is too much code to be written in the definition. I'd rather see something like what you see in RenderObject::preservesNewline() where you state it's inline, implement it in the header, but have the implementation at the end of the file.

> Source/WebCore/rendering/InlineBox.h:296
> +    EVerticalAlign verticalAlign() const
> +    {
> +        RenderBoxModelObject* object;
> +        if (m_renderer->isText())
> +            object = toRenderBoxModelObject(m_renderer->parent());
> +        else
> +            object = boxModelObject();
> +        return object->styleForFirstLine(m_bitfields.firstLine())->verticalAlign();
> +    }

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.