Bug 47841 – Rework baselinePosition and lineHeight to be writing-mode-aware.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Rework baselinePosition and lineHeight to be writing-mode-aware.

[Dave Hyatt]

Created attachment 71067 [details]
Patch

Comment on attachment 71067 [details]
Patch

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

> WebCore/rendering/InlineBox.h:25
>  #include "RenderBoxModelObject.h"
> +#include "RenderBR.h"

R comes before o.

> WebCore/rendering/RenderBlock.cpp:5139
> +        bool ignoreBaseline = (layer() && (layer()->marquee() || (direction == HorizontalLine ? (layer()->verticalScrollbar() || layer()->scrollYOffset() != 0)
> +            : (layer()->horizontalScrollbar() || layer()->scrollXOffset() != 0)))) || isWritingModeRoot();
> +        int baselinePos = ignoreBaseline ? -1 : lastLineBoxBaseline();
> +        
> +        int bottomOfContent = direction == HorizontalLine ? borderTop() + paddingTop() + contentHeight() : borderRight() + paddingRight() + contentWidth();
> +        if (baselinePos != -1 && baselinePos <= bottomOfContent)
> +            return direction == HorizontalLine ? marginTop() + baselinePos : marginRight() + baselinePos;
> +            
> +        return RenderBox::baselinePosition(firstLine, direction, linePositionMode);

I think you can easily get rid of the -1 and just use the boolean.

> WebCore/rendering/RenderBlock.cpp:5179
> +            const Font& f = style(true)->font();

I prefer firstLineStyle() here, so people don’t have to guess what 'true' means.

> WebCore/rendering/RenderBlock.cpp:5196
> +            const Font& f = style(true)->font();

Ditto

> WebCore/rendering/RenderSlider.cpp:158
> +int RenderSlider::baselinePosition(bool /*firstLine*/, LineDirectionMode /*direction*/, LinePositionMode /*linePositionMode*/) const

Don’t need the 2nd and 3rd commented-out parameter names.

[Dave Hyatt]

Fixed in r70072.

[WebKit Review Bot]

http://trac.webkit.org/changeset/70072 might have broken SnowLeopard Intel Release (Tests)