[Webkit-unassigned] [Bug 120769] [CSSRegions] 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





--- Comment #4 from Mihnea Ovidenie <mihnea at adobe.com>  2013-09-17 09:20:59 PST ---
(From update of attachment 211435)
View in context: https://bugs.webkit.org/attachment.cgi?id=211435&action=review

> Source/WebCore/rendering/RenderBlock.cpp:4483
>      return createVisiblePosition(0, DOWNSTREAM);

I would like to have someone with much more experience review the approach but i would also like to see a real patch, with layout tests and explanations.

> Source/WebCore/rendering/RenderBlock.cpp:4486
> +static inline bool isChildHitTestCandidate(RenderBox* box, RenderRegion *region, const LayoutPoint& point)

Why do you need to pass the region and the point here? Please note the the box may not always be a block, in which case you will hit a security assertion. Is the region check always required or only for the content that is inside the flow thread?

> Source/WebCore/rendering/RenderRegion.cpp:66
> +bool RenderRegion::isChildHitTestCandidate(RenderBox* box, const LayoutPoint& point)

The same function in RenderBlock is made static, basically the functions are the same. Again, the same observation about the cast from box to block. In fact, with the patch applied, it crashed my build.

> Source/WebCore/rendering/RenderRegion.cpp:72
> +VisiblePosition RenderRegion::positionForPoint(const LayoutPoint& point)

In the following patch, i would like to see a more detailed comment about what this function is trying to achieve, it will help review the approach/patch.

> Source/WebCore/rendering/RenderRegion.cpp:75
> +    if (thread->firstChild()) {

What are you trying to test here?

> Source/WebCore/rendering/RenderRegion.cpp:76
> +        RenderBox* candidate = toRenderBlock(thread->firstChild())->lastChildBox();

Again, if the first child of the thread is a replaced element, the conversion to block fails.

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