Bug 78772

Summary:

flex-wrap:nowrap should be flex-wrap:none

Product:

WebKit

Reporter:

Ojan Vafai <ojan>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

antaryami.pandia, dglazkov, joethomas, kennyluck, noel.gordon, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

62048

Attachments:

Description	Flags
ProposedPatch	ojan: review+, webkit.review.bot: commit-queue-
Patch-with-testcases-updated	none

Ojan Vafai

Reported 2012-02-15 19:18:09 PST

The spec changed to be consistent with text-wrap instead of white-space. http://dev.w3.org/csswg/css3-flexbox/#flex-wrap0

Attachments
ProposedPatch (17.32 KB, patch) 2012-02-16 16:54 PST, Joe Thomas	ojan: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch-with-testcases-updated (29.08 KB, patch) 2012-02-16 20:44 PST, Joe Thomas	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Joe Thomas

Comment 1 2012-02-16 16:54:04 PST

Created attachment 127470 [details] ProposedPatch

Ojan Vafai

Comment 2 2012-02-16 16:56:35 PST

Comment on attachment 127470 [details] ProposedPatch View in context: https://bugs.webkit.org/attachment.cgi?id=127470&action=review Thanks for the fix! > Source/WebCore/ChangeLog:10 > + Modified the existing test case. FYI: In the future you don't need to include this description. You can just delete the no new tests line. In cases where there actually isn't test coverage *and* you're not adding tests, then you do need an explanation of why tests aren't needed.

Joe Thomas

Comment 3 2012-02-16 16:58:44 PST

(In reply to comment #2) > (From update of attachment 127470 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127470&action=review > > Thanks for the fix! > > > Source/WebCore/ChangeLog:10 > > + Modified the existing test case. > > FYI: In the future you don't need to include this description. You can just delete the no new tests line. In cases where there actually isn't test coverage *and* you're not adding tests, then you do need an explanation of why tests aren't needed. Sure. Thanks for the review.

noel gordon

Comment 4 2012-02-16 19:16:31 PST

From http://queues.webkit.org/results/11536390, do we expect these tests to change, or was it just noise? Regressions: Unexpected text diff mismatch : (3) fast/css/getComputedStyle/computed-style-without-renderer.html = TEXT fast/css/getComputedStyle/computed-style.html = TEXT svg/css/getComputedStyle-basic.xhtml = TEXT

WebKit Review Bot

Comment 5 2012-02-16 19:42:47 PST

Comment on attachment 127470 [details] ProposedPatch Rejecting attachment 127470 [details] from commit-queue. New failing tests: svg/css/getComputedStyle-basic.xhtml fast/css/getComputedStyle/computed-style.html fast/css/getComputedStyle/computed-style-without-renderer.html Full output: http://queues.webkit.org/results/11535402

WebKit Review Bot

Comment 6 2012-02-16 20:21:54 PST

Comment on attachment 127470 [details] ProposedPatch Attachment 127470 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11543026 New failing tests: svg/css/getComputedStyle-basic.xhtml fast/css/getComputedStyle/computed-style.html fast/css/getComputedStyle/computed-style-without-renderer.html

Joe Thomas

Comment 7 2012-02-16 20:22:50 PST

(In reply to comment #4) > From http://queues.webkit.org/results/11536390, do we expect these tests to change, or was it just noise? > > Regressions: Unexpected text diff mismatch : (3) > fast/css/getComputedStyle/computed-style-without-renderer.html = TEXT > fast/css/getComputedStyle/computed-style.html = TEXT > svg/css/getComputedStyle-basic.xhtml = TEXT Yes these test cases needs to be updated. I will submit another patch soon.

Joe Thomas

Comment 8 2012-02-16 20:44:05 PST

Created attachment 127511 [details] Patch-with-testcases-updated Updated the failing test cases.

WebKit Review Bot

Comment 9 2012-02-17 11:56:58 PST

Comment on attachment 127511 [details] Patch-with-testcases-updated Clearing flags on attachment: 127511 Committed r108102: <http://trac.webkit.org/changeset/108102>

WebKit Review Bot

Comment 10 2012-02-17 11:57:03 PST

All reviewed patches have been landed. Closing bug.

noel gordon

Comment 11 2012-02-17 13:33:20 PST

(In reply to comment #7) > Yes these test cases needs to be updated. I will submit another patch soon. Thanks Joe. Odd that we need platform-specific results for dumpAsText() tests.

Note You need to log in before you can comment on or make changes to this bug.