[CSS Exclusions] Shape-inside content area may be larger than default content area

[Bear Travis]

The shape-inside content area is currently clipped to the default content area.

[Zoltan Horvath]

Created attachment 172882 [details]
proposed patch

[Bear Travis]

Comment on attachment 172882 [details]
proposed patch

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

The patch only tests a shape inside larger than the content area on the right/bottom sides. It also needs to deal with the upper/left sides. Check out the beginning of layoutRunsAndFloatsInRange and the setLogicalHeight call for how the top is set now. Also, the simple-rectangle.js script is designed to cover these basic test cases. I know that some of the changes here may break the simple float test case, but you might remove that test case to be added at a later date, since making floats work with shape-inside will require significant work.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:165
>          m_left = max<float>(m_segment->logicalLeft, m_left);
> -        m_right = min<float>(m_segment->logicalRight, m_right);
> +
> +        if (!(m_block->m_floatingObjects && m_block->m_floatingObjects->hasRightObjects()) && m_segment->logicalRight > m_right)
> +            m_right = m_segment->logicalRight;
> +        else
> +            m_right = min<float>(m_segment->logicalRight, m_right);

Do you know that if there are right floating objects, they necessarily affect this line? I think that the floating objects set contains all of this block's floats, so they may not. I think these lines should just be:
m_left = m_segment->logicalLeft;
m_right = m_segment->logicalRight;

This leaves the issue of floats, but perhaps the better thing would be to pull out the float test and implement that in a subsequent patch.

> LayoutTests/fast/exclusions/shape-inside/shape-inside-with-larger-content-area.html:16
> +        -webkit-shape-inside: rectangle(0, 0, 200px, 200px);

You need to test the rectangle going beyond the bounds of each of the boxes (content, padding, border, margin). I think to start, just going outside the margin box would be sufficient. eg,
#shape-inside {
    position: absolute;
    left: 10px; top: 10px;
    width: 180px; height: 180px;
    -webkit-shape-inside: rectangle(-10px, -10px, 200px, 200px);
}
Although really, this should be wrapped into using resources/simple-rectangle.js, in which case you would include the custom styling:
#shape-inside {
    position: absolute;
    left: 10px; top: 10px;
}
and the script:
createRectangleTest('shape-inside', 'stylesheet', { width: 200, height: 200 }, { x: -10, y: -10, width: 200, height: 200 }, 'px', null);
// use null for the last argument if you want to leave shape-inside's content in place
You can also see
shape-inside-subsequent-blocks, and shape-inside-multiple-blocks-dynamic.
The scripts obviate the need for the 'helper' element.

[Zoltan Horvath]

Comment on attachment 172882 [details]
proposed patch

I removed the patch from the review queue since it needs some improvement.

Thanks for the comments, things are one step clearer know.

[Zoltan Horvath]

The spec has been changed. This bug is not valid anymore.