[webkit-reviews] review granted: [Bug 46500] Make positioned elements work with vertical text : [Attachment 84123] Part 11: Make the static distance computations when left/right/top/bottom are auto writing mode aware.

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Dave Hyatt <hyatt at apple.com>'s request for review:
Bug 46500: Make positioned elements work with vertical text
https://bugs.webkit.org/show_bug.cgi?id=46500

Attachment 84123: Part 11: Make the static distance computations when
left/right/top/bottom are auto writing mode aware.
https://bugs.webkit.org/attachment.cgi?id=84123&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=84123&action=review

I am a little confused by the handling of static inline position for RTL
containing blocks. Maybe it was already wrong, but it looks like it’s changing.
Aren’t there tests covering this?

> Source/WebCore/rendering/RenderBox.cpp:2461
> +    int staticTop = child->layer()->staticBlockPosition() -
containerBlock->borderBefore();

May want to call this staticLogicalTop

> Source/WebCore/rendering/RenderFlexibleBox.cpp:420
> +			   child->layer()->setStaticBlockPosition(yPos);

Can use childLayer here too.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:679
> -		   if (child->style()->hasStaticX()) {
> -		       if (style()->isLeftToRightDirection())
> -			  
child->layer()->setStaticX(borderLeft()+paddingLeft());
> -		       else
> -			  
child->layer()->setStaticX(borderRight()+paddingRight());
> -		   }
> -		   if (child->style()->hasStaticY()) {
> +		   if
(child->style()->hasStaticInlinePosition(style()->isHorizontalWritingMode()))
> +		       child->layer()->setStaticInlinePosition(borderLeft() +
paddingLeft());

Was the RTL code wrong?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:683
> +			   child->layer()->setStaticBlockPosition(height());

Can use childLayer.