Bug 110341

Summary:

CSS Flexbox: dynamically applied align-items doesn't affect item alignment

Product:

WebKit

Reporter:

Masataka Yakura <myakura.web>

Component:

Layout and Rendering

Assignee:

Tony Chang <tony>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cbiesinger, eric, esprehn+autocc, ojan.autocc, ojan, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

62048

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Masataka Yakura

Reported 2013-02-20 06:40:23 PST

Steps to reproduce: 1. go to http://jsfiddle.net/gegJt/embedded/result/ 2. hit the button below the words "Item 1" and "Item 2" 3. check if the layout changes Expected result: Words "Item 1" and "Item 2" are horizontally aligned (looking "Item 1Item 2") Actual result: Words weren't aligned unless resizing window.

Attachments
Patch (8.65 KB, patch) 2013-02-26 11:51 PST, Tony Chang	no flags	Details Formatted Diff Diff
Patch for landing (8.88 KB, patch) 2013-02-26 12:24 PST, Tony Chang	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Ojan Vafai

Comment 1 2013-02-25 18:33:15 PST

Definitely a bug. Not sure where though. We do check webkit-align-items in RenderStyle::diff.

Masataka Yakura

Comment 2 2013-02-26 04:53:18 PST

(In reply to comment #1) > Definitely a bug. Not sure where though. We do check webkit-align-items in RenderStyle::diff. Perhaps somthing on text layout? It works fine on a modified testcase that set width and height explicitly. http://jsfiddle.net/suW5F/embedded/result/

Tony Chang

Comment 3 2013-02-26 10:59:02 PST

I see the bug. I'm working on a fix.

Tony Chang

Comment 4 2013-02-26 11:51:28 PST

Created attachment 190336 [details] Patch

Ojan Vafai

Comment 5 2013-02-26 12:03:19 PST

Comment on attachment 190336 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190336&action=review > Source/WebCore/ChangeLog:10 > + The problem was with flex items that were previously stretch but didn't change width, > + we would not lay them out again. Since they were not layed out, we use the old stretch > + value. Fix this by marking flex items that were stretch as needing layout. This grammar is a bit broken. I see what you're saying, but might be worth cleaning up. > Source/WebCore/rendering/RenderFlexibleBox.cpp:316 > + if (oldStyle && oldStyle->alignItems() == AlignStretch && diff == StyleDifferenceLayout) { > + for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) { > + EAlignItems previousAlignment = resolveAlignment(oldStyle, child->style()); > + if (previousAlignment == AlignStretch && previousAlignment != resolveAlignment(style(), child->style())) > + child->setChildNeedsLayout(true, MarkOnlyThis); > + } > + } I think this is correct, but it's tricky enough that it could use a brief "why" comment, e.g. why we only care about the alignment going from stretch to something else.

Tony Chang

Comment 6 2013-02-26 12:24:58 PST

Created attachment 190339 [details] Patch for landing

WebKit Review Bot

Comment 7 2013-02-26 14:00:29 PST

Comment on attachment 190339 [details] Patch for landing Clearing flags on attachment: 190339 Committed r144104: <http://trac.webkit.org/changeset/144104>

WebKit Review Bot

Comment 8 2013-02-26 14:00:33 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.