RESOLVED FIXED 110933
REGRESSION (r143643): Buttons containing floats render differently 
https://bugs.webkit.org/show_bug.cgi?id=110933
Summary REGRESSION (r143643): Buttons containing floats render differently
Christian Biesinger
Reported 2013-02-26 18:31:45 PST
See attached testcase. It looks like converting buttons to new-flexbox made it behave differently with respect to floats.
Attachments
testcase (736 bytes, text/html)
2013-02-26 18:32 PST, Christian Biesinger 		no flags
Details
Patch (4.97 KB, patch)
2013-02-27 14:08 PST, Christian Biesinger 		no flags
DetailsFormatted DiffDiff
Patch (7.98 KB, patch)
2013-02-27 14:24 PST, Christian Biesinger 		no flags
DetailsFormatted DiffDiff
Testcase for floats inside a flexitem (1.13 KB, text/html)
2013-03-04 12:11 PST, Christian Biesinger 		no flags
Details
Patch (8.17 KB, patch)
2013-03-04 14:05 PST, Christian Biesinger 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Christian Biesinger
Comment 1 2013-02-26 18:32:38 PST
Created attachment 190409 [details] testcase Firefox renders like old-webkit
Simon Fraser (smfr)
Comment 2 2013-02-27 11:22:50 PST
What revision did this regress in? (i.e. qualify the "now" in the title).
Christian Biesinger
Comment 3 2013-02-27 11:23:56 PST
It was bug 109994 aka r143643, on 2013-02-21 12:58:04
Christian Biesinger
Comment 4 2013-02-27 13:53:18 PST
Note, this also affects twitter.com (the "New Tweet" button in the topright corner)
Christian Biesinger
Comment 5 2013-02-27 14:08:35 PST
Created attachment 190601 [details] Patch
Christian Biesinger
Comment 6 2013-02-27 14:21:12 PST
This patch does fix the problem, but I'd appreciate opinions on whether this is the right fix. I could instead add isRenderButton() to the if. Come to think of it, I should also add a test to css3/flexbox that doesn't use a button. New patch coming...
Christian Biesinger
Comment 7 2013-02-27 14:24:37 PST
Created attachment 190602 [details] Patch
Zan Dobersek
Comment 8 2013-03-02 00:25:15 PST
*** Bug 111241 has been marked as a duplicate of this bug. ***
Christian Biesinger
Comment 9 2013-03-04 12:11:46 PST
Created attachment 191286 [details] Testcase for floats inside a flexitem
Christian Biesinger
Comment 10 2013-03-04 13:10:51 PST
OK, including new-flexbox in this if is correct because flex boxes establish a formatting context which means they should expand to include floats.
Ojan Vafai
Comment 11 2013-03-04 13:39:49 PST
Comment on attachment 190602 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190602&action=review > Source/WebCore/ChangeLog:13 > + Also include new flexbox as a renderer that needs to enclose > + overhanging floats. Might be worth an extra sentence here to mention that the spec says that flexboxes establish a block formatting context.
Christian Biesinger
Comment 12 2013-03-04 14:05:31 PST
Created attachment 191306 [details] Patch
Christian Biesinger
Comment 13 2013-03-04 14:11:19 PST
*** Bug 111263 has been marked as a duplicate of this bug. ***
WebKit Review Bot
Comment 14 2013-03-04 18:48:46 PST
Comment on attachment 191306 [details] Patch Clearing flags on attachment: 191306 Committed r144706: <http://trac.webkit.org/changeset/144706>
WebKit Review Bot
Comment 15 2013-03-04 18:48:50 PST
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.