Created attachment 217482 [details] Test case ShapeOutsideInfo is currently using its element's writing mode to setup the shape and compute line offsets. This needs to be done in writing mode of the parent of the line box instead.
Created attachment 218523 [details] Patch
Comment on attachment 218523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218523&action=review > Source/WebCore/rendering/shapes/ShapeInfo.cpp:63 > + WritingMode writingMode = m_renderer.isFloating() ? m_renderer.containingBlock()->style().writingMode() : m_renderer.style().writingMode(); You actually need to switch on it being shape-outside versus shape-inside because we can have the situation where m_renderer is floating and we want to compute the shape-inside for it's content, and in that case, we need to use m_renderer's writing mode, not that of the parent.
You may want to factor out a virtual method for ShapeOutsideInfo vs ShapeInsideInfo to solve the problem Bem mentioned. Also, you don't always want the containing block's writing mode. For example: <div> <div style='shape-outside: content-box; float: left;'></div> <div style='writing-mode: vertical-lr'>I will be affected by the shape</div> </div> You may also have overhanging floats: <div> <div style='shape-outside: content-box; float: left;'>I am bigger than my container</div> </div> <div style='writing-mode: vertical-lr'>I will be affected by the shape</div> So you'll need to work out a transformation between the writing mode of the inline content, and whatever coordinate system the float is using. I think it is probably more than one patch.
(In reply to comment #3) > You may want to factor out a virtual method for ShapeOutsideInfo vs ShapeInsideInfo to solve the problem Bem mentioned. > > Also, you don't always want the containing block's writing mode. For example: > <div> > <div style='shape-outside: content-box; float: left;'></div> > <div style='writing-mode: vertical-lr'>I will be affected by the shape</div> > </div> > > You may also have overhanging floats: > <div> > <div style='shape-outside: content-box; float: left;'>I am bigger than my container</div> > </div> > <div style='writing-mode: vertical-lr'>I will be affected by the shape</div> > > So you'll need to work out a transformation between the writing mode of the inline content, and whatever coordinate system the float is using. I think it is probably more than one patch. Bear, your example is incorrect; a writing mode establishes a block formatting context, and floats don't cross BFCs, so the float won't affect the content in your example. Overhanging/intruding floats also can only affect content with the same writing mode as their containing block, so checking the writing mode on the containing block is sufficient.
Created attachment 218527 [details] Patch
Created attachment 218537 [details] Patch
Comment on attachment 218537 [details] Patch Attachment 218537 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/44298010 New failing tests: inspector-protocol/model/highlight-shape-outside.html
Created attachment 218545 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 218547 [details] Patch
Comment on attachment 218547 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218547&action=review I would like to hear Bems or Bears opinion before r=me. At the moment I do not understand what this change is about. > Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp:106 > + if (m_renderer.isFloating()) { > + if (m_renderer.containingBlock()) this can be if (m_renderer.isFloating() && m_renderer.containingBlock()) return m_renderer.containingBlock()->style().writingMode();
Comment on attachment 218547 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218547&action=review >> Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp:106 >> + if (m_renderer.containingBlock()) > > this can be > > if (m_renderer.isFloating() && m_renderer.containingBlock()) > return m_renderer.containingBlock()->style().writingMode(); ShapeOutside shouldn't ever use the internal writing mode, since it always affects content outside of the block, so I'd remove the isFloating() check. As for the containingBlock() check: will we ever be in a situation where we have a valid shape-outside and don't have a containingBlock? I'd lean more towards an ASSERT for this and to remove the if statement entirely. > LayoutTests/TestExpectations:72 > +# A problem in both EFL and GTK. Doesn't this fail on Mac as well after this patch? Perhaps just remove the comment?
The test passes on Mac, the root of the problem is whether rectangle paths contain the last line segment or omit it. It's a platform dependent issue (yay). Looks fine to me.
Created attachment 218611 [details] Patch
Comment on attachment 218611 [details] Patch Clearing flags on attachment: 218611 Committed r160243: <http://trac.webkit.org/changeset/160243>
All reviewed patches have been landed. Closing bug.