Bug 117573

Summary: [CSS Shapes] Shape's content gets extra left offset when left-border is positive on the content box
Product: WebKit Reporter: Zoltan Horvath <zoltan>
Component: CSSAssignee: Bear Travis <betravis>
Status: RESOLVED FIXED    
Severity: Normal CC: betravis, commit-queue, esprehn+autocc, glenn, kondapallykalyan, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89256    
Attachments:
Description Flags
Initial patch
none
Updated patch
none
Updating changelogs
none
Updated patch
none
Updated patch none

Zoltan Horvath
Reported 2013-06-12 17:53:53 PDT
I will attach a test file later.
Attachments
Initial patch (16.61 KB, patch)
2013-07-11 12:03 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updated patch (17.94 KB, patch)
2013-07-23 11:50 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updating changelogs (17.98 KB, patch)
2013-07-23 13:24 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updated patch (17.82 KB, patch)
2013-08-15 11:45 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Updated patch (17.74 KB, patch)
2013-08-30 11:31 PDT, Bear Travis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Bear Travis
Comment 1 2013-06-17 12:39:16 PDT
I think this may be related to bug 110475
Bear Travis
Comment 2 2013-07-11 12:03:24 PDT
Created attachment 206482 [details] Initial patch
Dave Hyatt
Comment 3 2013-07-12 10:46:24 PDT
Comment on attachment 206482 [details] Initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=206482&action=review r- > Source/WebCore/rendering/RenderBlock.cpp:1439 > +LayoutSize RenderBlock::offsetFromShapeAncestorContainer(const RenderBlock* container) const Please rename this to logicalOffsetFromShapeAncestorContainer to reflect that you're putting the y into the width() variable in vertical writing modes. > Source/WebCore/rendering/RenderBlock.cpp:1442 > + LayoutRect blockRect(0, 0, currentBlock->width(), currentBlock->height()); This is borderBoxRect(), so you can initialize to that, e.g., LayoutRect blockRect(currentBlock->borderBoxRect()); > Source/WebCore/rendering/RenderBlock.cpp:1450 > + LayoutPoint currentBlockLocation = currentBlock->location(); > + > + blockRect.moveBy(currentBlockLocation); This doesn't work when you cross writing mode boundaries. Remember that block coordinates can be flipped. When you cross into writing modes with different flips, you have to adjust accordingly. You'll want to test with mixed vertical-rl and vertical-lr and/or horizontal-tb/horizontal-bt to see what I mean. > Source/WebCore/rendering/RenderBlock.cpp:2741 > + LayoutSize childOffset = child->location() - oldRect.location(); > + if (childOffset.width() && childRenderBlock && layoutShapeInsideInfo() && !childRenderBlock->shapeInsideInfo()) { > + childRenderBlock->setNormalChildNeedsLayout(true); > + childRenderBlock->markShapeInsideDescendantsForLayout(); > + childRenderBlock->layoutIfNeeded(); > + } Put this in a helper function. Also, why are you only checking width? Is the intent to look for a change in logical left position? If so, you need to check height() if vertical. > Source/WebCore/rendering/RenderBlock.h:591 > + LayoutSize offsetFromShapeAncestorContainer(const RenderBlock* container) const; logicalOffset... > Source/WebCore/rendering/RenderBlockLineLayout.cpp:1688 > + shapeInsideInfo->computeSegmentsForLine(lineLeft, lineTop, lineHeight); Use a LayoutSize rather than two arguments here. > Source/WebCore/rendering/shapes/ShapeInsideInfo.h:71 > + bool computeSegmentsForLine(LayoutUnit lineLeft, LayoutUnit lineTop, LayoutUnit lineHeight) Why not combine lineLeft/lineTop into a LayoutSize offset?
Bear Travis
Comment 4 2013-07-23 11:50:34 PDT
Created attachment 207341 [details] Updated patch
Bear Travis
Comment 5 2013-07-23 13:24:09 PDT
Created attachment 207344 [details] Updating changelogs
Bear Travis
Comment 6 2013-08-15 11:45:57 PDT
Created attachment 208828 [details] Updated patch
Build Bot
Comment 7 2013-08-23 08:08:32 PDT
Comment on attachment 208828 [details] Updated patch Attachment 208828 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1544282
Bear Travis
Comment 8 2013-08-30 11:31:41 PDT
Created attachment 210139 [details] Updated patch
Dave Hyatt
Comment 9 2013-09-03 14:47:06 PDT
Comment on attachment 210139 [details] Updated patch r=me
WebKit Commit Bot
Comment 10 2013-09-03 15:13:51 PDT
Comment on attachment 210139 [details] Updated patch Clearing flags on attachment: 210139 Committed r155002: <http://trac.webkit.org/changeset/155002>
WebKit Commit Bot
Comment 11 2013-09-03 15:13:54 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.