[Webkit-unassigned] [Bug 120769] [CSS Regions] Selection focusNode set to the "region" block, instead of the "source" block

[bugzilla-daemon at webkit.org]

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


Dave Hyatt <hyatt at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #214161|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #16 from Dave Hyatt <hyatt at apple.com>  2013-10-14 11:02:33 PST ---
(From update of attachment 214161)
View in context: https://bugs.webkit.org/attachment.cgi?id=214161&action=review

r- for the writing mode issues. I want a vertical writing mode test added for the next patch because of this.

> Source/WebCore/rendering/RenderBlock.cpp:3644
> +    const RenderBlock &block = box.isRenderBlock() ? toRenderBlock(box) : *box.containingBlock();

Surprised the style code didn't complain on this line. Move the & to be next to RenderBlock, i.e.,

const RenderBlock& block

> Source/WebCore/rendering/RenderRegion.cpp:72
> +    if (point.y() < 0)
> +        return LayoutPoint(0, m_flowThreadPortionRect.y());

Not writing-mode-aware. For vertical writing modes it is x you need to check.

> Source/WebCore/rendering/RenderRegion.cpp:74
> +    if (point.y() > m_flowThreadPortionRect.height()) {

Same problem.

> Source/WebCore/rendering/RenderRegion.cpp:82
> +    return LayoutPoint(point.x(), point.y() + m_flowThreadPortionRect.y());

Same problem, etc.

> Source/WebCore/rendering/RenderRegion.cpp:88
> +        return RenderBlock::positionForPoint(point);

Shouldn't this be RenderBlockFlow::positionForPoint?

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