Bug 100999 - [CSS Exclusions] Shape-inside content area may be larger than default content area
Summary: [CSS Exclusions] Shape-inside content area may be larger than default content...
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zoltan Horvath
Depends on:
Blocks: 89256
  Show dependency treegraph
Reported: 2012-11-01 16:42 PDT by Bear Travis
Modified: 2013-01-24 16:36 PST (History)
3 users (show)

See Also:

proposed patch (5.11 KB, patch)
2012-11-07 15:37 PST, Zoltan Horvath
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bear Travis 2012-11-01 16:42:02 PDT
The shape-inside content area is currently clipped to the default content area.
Comment 1 Zoltan Horvath 2012-11-07 15:37:18 PST
Created attachment 172882 [details]
proposed patch
Comment 2 Bear Travis 2012-11-07 22:08:28 PST
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.
Comment 3 Zoltan Horvath 2012-11-07 22:54:49 PST
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.
Comment 4 Zoltan Horvath 2013-01-24 16:36:57 PST
The spec has been changed. This bug is not valid anymore.