Bug 78772 – flex-wrap:nowrap should be flex-wrap:none

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ojan Vafai]

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

[Joe Thomas]

Created attachment 127470 [details]
ProposedPatch

[Ojan Vafai]

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]

(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]

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 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 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]

(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]

Created attachment 127511 [details]
Patch-with-testcases-updated

Updated the failing test cases.

[WebKit Review Bot]

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]

All reviewed patches have been landed.  Closing bug.

[noel gordon]

(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.