Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for whitespace: nowrap and float:left #9605

Merged

Conversation

Projects
None yet
7 participants
@stephenmcgruer
Copy link
Contributor

stephenmcgruer commented Feb 21, 2018

When whitespace: nowrap is specified, text should not wrap unless it is
forced to. This includes if content is floated; the float should be placed
before the content on the same line.

This test comes from
https://bugzilla.mozilla.org/show_bug.cgi?id=488725, as Firefox does not
currently follow the spec correctly.

@stephenmcgruer

This comment has been minimized.

Copy link
Contributor Author

stephenmcgruer commented Feb 21, 2018

I'm not sure this is the right place for this test; it may be better suited to a float test (or a test may already exist for it? I was unable to find such a test). Happy to change things up as required.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 21, 2018

Build PASSED

Started: 2018-03-14 14:44:33
Finished: 2018-03-14 14:55:26

View more information about this build on:

@stephenmcgruer

This comment has been minimized.

Copy link
Contributor Author

stephenmcgruer commented Feb 28, 2018

@mstensho Phillip mentioned you may be the right person to review this? If not apologies for the ping.

@mstensho
Copy link
Contributor

mstensho left a comment

Typo in commit message: "cotent".
Also, it claims that container width could force line breaks even if whitespace is nowrap. That's wrong.

position: relative;
color:green;
line-height: 1em;
font-family: ahem;

This comment has been minimized.

Copy link
@mstensho

mstensho Feb 28, 2018

Contributor

Ahem, not ahem. :) Makes a difference for reasons not obvious to me. See https://bugs.chromium.org/p/chromium/issues/detail?id=724392

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 7, 2018

Author Contributor

Done.

div {
color:green;
line-height: 1em;
font-family: ahem;

This comment has been minimized.

Copy link
@mstensho

mstensho Feb 28, 2018

Contributor

Ahem.

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 7, 2018

Author Contributor

Done.

<meta charset="utf-8">
<title>CSS Text — line breaking for nowrap and floats</title>
<meta name=assert content="When whitespace: nowrap is specified, floats should not cause line breaks">
<link rel=help href="https://www.w3.org/TR/css-text-3/#white-space-property">

This comment has been minimized.

Copy link
@mstensho

mstensho Feb 28, 2018

Contributor

Author missing?

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 7, 2018

Author Contributor

I don't believe that author is required (@foolip can confirm)

This comment has been minimized.

Copy link
@mstensho

mstensho Mar 7, 2018

Contributor

Yeah, I think you're right, actually. Sorry about the noise.

This comment has been minimized.

Copy link
@foolip

foolip Mar 8, 2018

Contributor

I confirm :)

<html>
<meta charset="utf-8">
<title>Reference File for line breaking tests</title>
<link rel=author title="Florian Rivoal" href="http://florian.rivoal.net">

This comment has been minimized.

Copy link
@mstensho

mstensho Feb 28, 2018

Contributor

Did Florian write this?

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 7, 2018

Author Contributor

Whoops, accidental copy/paste error. Removed.

<title>CSS Text — line breaking for nowrap and floats</title>
<meta name=assert content="When whitespace: nowrap is specified, floats should not cause line breaks">
<link rel=help href="https://www.w3.org/TR/css-text-3/#white-space-property">
<link rel=match href="reference/line-breaking-002-ref.html">

This comment has been minimized.

Copy link
@mstensho

mstensho Feb 28, 2018

Contributor

Why is it 002-ref, when this one is 012?

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 7, 2018

Author Contributor

The current set of line-breaking tests seem to be setup so that they all match the same reference file (css/css-text/line-breaking/reference/line-breaking-001-ref.html). This test needs a different reference as 001-ref.html tests when a line break does occur whilst this checks that one doesn't occur. So 002-ref.html :).

This comment has been minimized.

Copy link
@frivoal

frivoal Mar 8, 2018

Contributor

It is common practice to number the ref after the first test that matches it, so this would be -012-ref, even though there's not -002 through -011. Not a blocker though.

This comment has been minimized.

Copy link
@stephenmcgruer

stephenmcgruer Mar 14, 2018

Author Contributor

Ack, moved to -012-ref

stephenmcgruer added some commits Feb 21, 2018

Add test for whitespace: nowrap and float:left
When whitespace: nowrap is specified, text should not wrap unless it is
forced to (e.g. by the width of the container, a break, etc). This
includes if content is floated; the float should just move before the
cotent on the same line.

This test comes from
https://bugzilla.mozilla.org/show_bug.cgi?id=488725, as it turns out
that Firefox does not follow the spec correctly here.

@stephenmcgruer stephenmcgruer force-pushed the stephenmcgruer:whitespace_float_left branch from cf1899c to 4b10fa9 Mar 7, 2018

@stephenmcgruer

This comment has been minimized.

Copy link
Contributor Author

stephenmcgruer commented Mar 7, 2018

Fixed the commit message too

@mstensho
Copy link
Contributor

mstensho left a comment

I'm too unfamiliar with this review tool, but I can't seem to find the updated commit message. But if you fixed what I pointed out there, this is lgtm.

@Hexcles Hexcles merged commit 7d1bf74 into web-platform-tests:master Mar 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stephenmcgruer stephenmcgruer deleted the stephenmcgruer:whitespace_float_left branch Mar 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.