RESOLVED FIXED Bug 110372
[CSS Exclusions] Properly position multiple stacked floats with non rectangular shape outside
https://bugs.webkit.org/show_bug.cgi?id=110372
Summary [CSS Exclusions] Properly position multiple stacked floats with non rectangul...
Bem Jones-Bey
Reported 2013-02-20 13:23:09 PST
With a non-rectangular shape-outside, stacked floats should use the bounding box of previous floats to position.
Attachments
Patch (23.69 KB, patch)
2013-04-08 11:32 PDT, Bem Jones-Bey
dino: review+
Patch (23.65 KB, patch)
2013-04-09 13:49 PDT, Bem Jones-Bey
commit-queue: commit-queue-
Patch (23.65 KB, patch)
2013-04-09 14:07 PDT, Bem Jones-Bey
no flags
Bem Jones-Bey
Comment 1 2013-04-08 11:32:15 PDT
Created attachment 196877 [details] Patch Fix stacked float handling
Dean Jackson
Comment 2 2013-04-09 11:41:16 PDT
Comment on attachment 196877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196877&action=review > Source/WebCore/ChangeLog:22 > + (WebCore::RenderBlock::logicalRightOffsetForLine): Add paramter for offset mode. Typo: parameter > Source/WebCore/rendering/RenderBlockLineLayout.cpp:202 > + RenderBlock::FloatingObjectSetIterator it = floatingObjectSet.end(); > + --it; // Go to last item. > + RenderBlock::FloatingObjectSetIterator begin = floatingObjectSet.begin(); > + while (it != begin) { > + --it; Doesn't this skip the very last item in the first pass? Is that ok?
Bem Jones-Bey
Comment 3 2013-04-09 11:49:00 PDT
(In reply to comment #2) > (From update of attachment 196877 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=196877&action=review > > > Source/WebCore/ChangeLog:22 > > + (WebCore::RenderBlock::logicalRightOffsetForLine): Add paramter for offset mode. > > Typo: parameter Thanks, I'll fix that. > > > Source/WebCore/rendering/RenderBlockLineLayout.cpp:202 > > + RenderBlock::FloatingObjectSetIterator it = floatingObjectSet.end(); > > + --it; // Go to last item. > > + RenderBlock::FloatingObjectSetIterator begin = floatingObjectSet.begin(); > > + while (it != begin) { > > + --it; > > Doesn't this skip the very last item in the first pass? Is that ok? I was thinking that would be ok, since our new float will always be the last one on the list the way that the code is currently used, but I should be paranoid and not skip the last one just in case I'm missing something.
Bem Jones-Bey
Comment 4 2013-04-09 13:49:41 PDT
Created attachment 197167 [details] Patch Update for review comments
WebKit Commit Bot
Comment 5 2013-04-09 13:53:23 PDT
Comment on attachment 197167 [details] Patch Rejecting attachment 197167 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--non-interactive', 197167, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-commit-queue.appspot.com/results/17628013
Bem Jones-Bey
Comment 6 2013-04-09 14:07:37 PDT
Created attachment 197169 [details] Patch Update for review comments
Zoltan Horvath
Comment 7 2013-04-09 14:08:57 PDT
Comment on attachment 197169 [details] Patch Somebody should teach the commit-queue to figure out the reviewer's name at these straightforward cases.
WebKit Commit Bot
Comment 8 2013-04-09 15:30:28 PDT
Comment on attachment 197169 [details] Patch Clearing flags on attachment: 197169 Committed r148056: <http://trac.webkit.org/changeset/148056>
WebKit Commit Bot
Comment 9 2013-04-09 15:30:30 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.