Bug 110349

Summary:

[css exclusions] overflow:hidden undoes shape-outside offsets

Product:

WebKit

Reporter:

Bear Travis <betravis>

Component:

CSS

Assignee:

Bem Jones-Bey <bjonesbe>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bjonesbe, eric, esprehn+autocc, gtk-ews, ojan.autocc, simon.fraser, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

http://codepen.io/btravis/pen/HuecB

Bug Depends on:

Bug Blocks:

98664

Attachments:

Description	Flags
Patch	jchaffraix: review+, jchaffraix: commit-queue-
Patch	none

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-	Details Formatted Diff Diff
Patch (5.64 KB, patch) 2013-04-02 10:40 PDT, Bem Jones-Bey	no flags	Details Formatted Diff Diff
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.