[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 #3 from Julien Chaffraix <jchaffraix at webkit.org>  2012-06-12 15:59:24 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
>> +    }
> 
> 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.

For the boolean, this is unfortunately the current state of our code where we get a boolean telling us if we are on the first line. I would have preferred more differentiated code paths but that's likely too difficult to implement as part of this change.

The naming could indeed love some massaging. It's more a firstLineAwareStyle() than really a styleForFirstLine().

> Source/WebCore/rendering/InlineTextBox.cpp:922
> -    renderer()->getTextDecorationColors(deco, underline, overline, linethrough, true, isFirstLineStyle());
> +    toRenderBoxModelObject(renderer()->parent())->getTextDecorationColors(deco, underline, overline, linethrough, true, isFirstLineStyle());

Looks like an change in behavior.

> Source/WebCore/rendering/RenderBR.cpp:45
> +        RenderStyle* s = toRenderBoxModelObject(parent())->styleForFirstLine(firstLine);

There is a lot of toRenderBoxModelObject(someRenderer)->styleForFirstLine. Could it be possible to reduce the number of such casts? One idea that pops up is to change helper functions to take a RenderBoxModelObject instead of a RenderObject.

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