Bug 97948

Summary: flexbox does wrong baseline item alignment in columns
Product: WebKit Reporter: Tony Chang <tony>
Component: Layout and RenderingAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 62048    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Tony Chang
Reported 2012-09-28 15:58:06 PDT
The spec says, "If the flex item's inline axis is the same as the cross axis, this value is identical to ‘flex-start’." We're not doing that. See https://bugs.webkit.org/attachment.cgi?id=163005 for some test cases.
Attachments
Patch (22.18 KB, patch)
2012-10-01 15:13 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (23.55 KB, patch)
2012-10-01 16:06 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (22.85 KB, patch)
2012-10-01 16:07 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2012-10-01 15:13:20 PDT
Created attachment 166555 [details] Patch
Ojan Vafai
Comment 2 2012-10-01 15:42:34 PDT
Comment on attachment 166555 [details] Patch What if, instead of changing the logic of AlignBaseline, we made the chunk before the switch statement pretend that we have a different alignment. That would reduce some code duplication and more clearly match the spec text. FlexAlign alignment = alignmentForChild(child); if (alignment == AlignBaseline and hasOrthogonalFlow(child)) alignment = (style()->flexWrap() == FlexWrapReverse) ? AlignFlexEnd : AlignFlexStart; Up to you. I find this logic easier to follow.
Tony Chang
Comment 3 2012-10-01 16:06:02 PDT
Created attachment 166563 [details] Patch
Tony Chang
Comment 4 2012-10-01 16:07:36 PDT
Created attachment 166564 [details] Patch
Tony Chang
Comment 5 2012-10-01 16:08:05 PDT
Comment on attachment 166564 [details] Patch Can you take another look? I moved the logic into alignmentForChild.
WebKit Review Bot
Comment 6 2012-10-01 16:47:57 PDT
Comment on attachment 166564 [details] Patch Rejecting attachment 166564 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: webkit-commit-queue/Source/WebKit/chromium/webkit --revision 159019 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 51>At revision 159019. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/14089966
Build Bot
Comment 7 2012-10-01 17:46:06 PDT
Comment on attachment 166564 [details] Patch Attachment 166564 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14084986 New failing tests: http/tests/workers/terminate-during-sync-operation.html
WebKit Review Bot
Comment 8 2012-10-01 18:51:11 PDT
Comment on attachment 166564 [details] Patch Clearing flags on attachment: 166564 Committed r130110: <http://trac.webkit.org/changeset/130110>
WebKit Review Bot
Comment 9 2012-10-01 18:51:14 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.