Summary: | [CSS Exclusions] Properly position multiple stacked floats with non rectangular shape outside | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Bem Jones-Bey <bjonesbe> | ||||||||
Component: | CSS | Assignee: | Bem Jones-Bey <bjonesbe> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, eric, esprehn+autocc, ojan.autocc, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
URL: | http://dev.w3.org/csswg/css3-exclusions/#floats-and-exclusions | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 98664 | ||||||||||
Attachments: |
|
Description
Bem Jones-Bey
2013-02-20 13:23:09 PST
Created attachment 196877 [details]
Patch
Fix stacked float handling
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? (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. Created attachment 197167 [details]
Patch
Update for review comments
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 Created attachment 197169 [details]
Patch
Update for review comments
Comment on attachment 197169 [details]
Patch
Somebody should teach the commit-queue to figure out the reviewer's name at these straightforward cases.
Comment on attachment 197169 [details] Patch Clearing flags on attachment: 197169 Committed r148056: <http://trac.webkit.org/changeset/148056> All reviewed patches have been landed. Closing bug. |