Bug 128631 – [CSS Shapes] shape-outside does not properly handle different writing modes

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Bem Jones-Bey]

ShapeOutsideInfo::updateDeltasForContainingBlockLine doesn't properly convert from the writing mode and coordinate space of the containing block to and from the coordinate space of the shape. Some of this is due to the confusing names of the variables involved, some is due to the fact that the logicalLeftOffset(), logicalTopOffset() and other methods in ShapeInfo use the writing mode of the float, not of the parent. This bug tracks fixing all of these issues.

[Bem Jones-Bey]

Created attachment 223909 [details]
Do not land this patch

Large patch for prelimary perusal. Real patch would be split into smaller pieces and include things like tests.

[Bem Jones-Bey]

Created attachment 224255 [details]
Patch

I tried breaking this up into multiple patches, but had trouble coming up with tests for each piece.

[Bem Jones-Bey]

Comment on attachment 224255 [details]
Patch

I've gotta add a couple of ASSERT_NOT_REACHED to make EFL happy.

[Dave Hyatt]

Comment on attachment 224255 [details]
Patch

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

r=me with a rename suggestion.

> Source/WebCore/rendering/RenderBoxModelObject.h:121
> +    virtual LayoutUnit borderWidth() const { return borderLeft() + borderRight(); }
> +    virtual LayoutUnit borderHeight() const { return borderTop() + borderBottom(); }

I would suggest naming these horizontalBorderExtent and verticalBorderExtent. I dislike using terms like "width" since it could be confused with the actual border-width CSS name, i.e., a person new to this code would think the method was returning the pixel width of a single border.

I think renaming all of the other functions would be good too, i.e., marginWidth = horizontalMarginExtent, marginHeight = verticalMarginExtent, etc.

borderAndPaddingWidth could be horizontalBorderAndPaddingExtent, and then borderAndPaddingHeight could be verticalBorderAndPaddingExtent.

[Bem Jones-Bey]

Created attachment 224536 [details]
Patch

Will do renames in a followup patch

[Bem Jones-Bey]

Created attachment 224537 [details]
Patch

forgot to update reviewer

[Bem Jones-Bey]

Created attachment 224560 [details]
Patch

My ability to read error messages is apparently a problem. I need a return statement to shut that EFL compiler up!

[WebKit Commit Bot]

Comment on attachment 224560 [details]
Patch

Clearing flags on attachment: 224560

Committed r164363: <http://trac.webkit.org/changeset/164363>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.