Refactor computeInlineStaticDistance()
Created attachment 108586 [details] Patch
Created attachment 108605 [details] Patch
Comment on attachment 108605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108605&action=review > Source/WebCore/rendering/RenderBox.cpp:2222 > + // See bug 50576. If the immediate parent in an RTL is a RenderInline > + // we need to use the width of the containing block. Comments need to say “why” things are true, not just repeat in words what the code says. This comment should be replaced by a “why” comment. I’m also not at all fond of comments that point to a bug report.
Created attachment 108702 [details] Patch
Comment on attachment 108702 [details] Patch I'm r-ing this patch because the function computeInlineStaticDistance() in RenderBox.cpp has significantly changed since this patch was proposed over 6 months ago. If this patch is still applicable then it will need to be rebased.
Comment on attachment 108702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108702&action=review More comments. > Source/WebCore/ChangeLog:17 > + (WebCore::RenderBox::computePositionedLogicalWidthReplaced): Ditto. Typo parameter. > Source/WebCore/rendering/RenderBox.cpp:2225 > + if (container()->isRenderInline()) > + return containingBlock()->width() - logicalRight(); > + return toRenderBox(container())->width() - logicalRight(); Couldn't this just be a toRenderBoxModelObject(container())->width() - logicalRight() ? (btw container and containingBlock are not interchangeable, there is a very small differences) > Source/WebCore/rendering/RenderBox.h:416 > + LayoutUnit startOffset(); Shouldn't it be const? > Source/WebCore/rendering/RenderBoxModelObject.h:98 > + LayoutUnit borderStart(RenderStyle* style) const { return style->isHorizontalWritingMode() ? (style->direction() == LTR) ? borderLeft() : borderRight() : (style->direction() == LTR) ? borderTop() : borderBottom(); } Could this be re-written to be less confusing?