RESOLVED FIXED Bug 110349
[css exclusions] overflow:hidden undoes shape-outside offsets 
https://bugs.webkit.org/show_bug.cgi?id=110349
Summary [css exclusions] overflow:hidden undoes shape-outside offsets
Bear Travis
Reported 2013-02-20 07:32:21 PST
Setting overflow:hidden, float: left, and shape-outside: rectangle does not offset correctly. See link for an example.
Attachments
Patch (5.70 KB, patch)
2013-03-29 14:13 PDT, Bem Jones-Bey 		jchaffraix: review+
jchaffraix: commit-queue-
DetailsFormatted DiffDiff
Patch (5.64 KB, patch)
2013-04-02 10:40 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Bem Jones-Bey
Comment 1 2013-03-18 15:21:20 PDT
It looks like the shape is still handled normally, but the offset for the content of the float is no longer handled properly when overflow is hidden. Still very strange, but it is comforting that the wrapping behavior doesn't change in this case.
Bem Jones-Bey
Comment 2 2013-03-29 14:13:42 PDT
Created attachment 195798 [details] Patch Fix layer handling for overflow hidden and shapes
kov's GTK+ EWS bot
Comment 3 2013-03-29 15:20:15 PDT
Comment on attachment 195798 [details] Patch Attachment 195798 [details] did not pass gtk-ews (gtk): Output: http://webkit-commit-queue.appspot.com/results/17369111
Bem Jones-Bey
Comment 4 2013-03-29 15:35:36 PDT
Comment on attachment 195798 [details] Patch Since the GTK build bots are completely broken right now and the GTK EWS bot's output doesn't have any error whatsoever that's related to my patch, I contend that it's failure is entirely unrelated to my patch. So I'm going to put the cq? back.
Julien Chaffraix
Comment 5 2013-04-01 14:52:13 PDT
Comment on attachment 195798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195798&action=review > Source/WebCore/rendering/RenderLayer.cpp:5850 > + && !(renderer()->style()->shapeOutside() && renderer()->isFloating()); RenderObject::isFloating is probably cheaper to evaluate so you would be better off changing the order. This starts to be a pattern in the code where you check renderer()->style()->shapeOutside() && renderer()->isFloating() so it should be refactored into its own helper method. > LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-hidden-expected.html:1 > +<html> No doctype! > LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-hidden-expected.html:8 > + background-color: red; Let's not put any red in the expected result as it would make the test pass, even if you are saying we shouldn't see any red. > LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-hidden.html:1 > +<html> Same comment.
Build Bot
Comment 6 2013-04-01 23:54:59 PDT
Comment on attachment 195798 [details] Patch Attachment 195798 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17405042
Bem Jones-Bey
Comment 7 2013-04-02 10:22:09 PDT
(In reply to comment #5) > This starts to be a pattern in the code where you check renderer()->style()->shapeOutside() && renderer()->isFloating() so it should be refactored into its own helper method. Doing this right involves a bunch more changes than I am confortable including in this patch right now, so I've filed Bug 113799 to do the refactoring. (I did the straightforward change, and introduced a crash, so I figure that I need to spend more time on it)
Bem Jones-Bey
Comment 8 2013-04-02 10:40:59 PDT
Created attachment 196176 [details] Patch Update for review comments
Max Vujovic
Comment 9 2013-04-02 10:47:11 PDT
Comment on attachment 196176 [details] Patch Setting cq+ for Bem.
WebKit Review Bot
Comment 10 2013-04-02 11:08:36 PDT
Comment on attachment 196176 [details] Patch Clearing flags on attachment: 196176 Committed r147463: <http://trac.webkit.org/changeset/147463>
WebKit Review Bot
Comment 11 2013-04-02 11:08:40 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.