User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040106 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040106 Mozilla DOES respond correctly to the CSS 2.1 white-space property for the values 'normal' and 'pre' and 'nowrap', but DOES NOT seem to respond at all to the values 'pre-wrap', or 'pre-line'. Reproducible: Always Steps to Reproduce: 1. Create a style sheet with an element given the property 'white-space: pre-wrap'. 2. Also create an element with the property 'white-space: pre-line'. 3. Display the new page, and compare the results to the results as described in the CSS 2.1 latest draft for text properties, under "Whitespace..." Actual Results: The result produced by Mozilla was not the result expected of 'pre-wrap', as whitespace was collapsed when it should not be. The result produced by Mozilla was not the result expected of 'pre-line', as whitespace is collapsed, but newlines are not observed. Expected Results: 'pre-wrap', according to the CSS 2.1 draft, "prevents user agents from collapsing sequences of whitespace. Lines are broken at newlines in the source, at occurrences of "\A" in generated content, and as necessary to fill line boxes." 'pre-line', according to the CSS 2.1 draft, "directs user agents to collapse sequences of whitespace. Lines are broken at newlines in the source, at occurrences of "\A" in generated content, and as necessary to fill line boxes."

Xref to bug 99457. Also bug 206152 is line breaking meta bug.

Note that those property values are new in CSS2.1 (they are not in CSS 2.0) and CSS2.1 is not yet in CR, so implementing its new properties without vendor prefixes is highly undesirable at this point. We implement -moz-pre-wrap (and have for ages), but we'd need to add pre-line stuff... Resummarizing to that effect and confirming.

Actually they're also in CSS3 Text and CSS3 Text is in CR so no prefixes are needed on these.

*** Bug 230986 has been marked as a duplicate of this bug. ***

The test case tests all the values 'white-space' can have according to CSS2.1. 'pre-line' and 'pre-wrap' have also a '-moz-' prefixed test.

CSS 2.1 is already in CR. http://www.w3.org/TR/2004/CR-CSS21-20040225/

As of yesterday, yes. And it'll stay there for 6 months or so, during which time they are calling for people to maybe implement it. I await your patches most eagerly.

*** Bug 261081 has been marked as a duplicate of this bug. ***

See also bug 261081.

I'll look into...hopefully

(In reply to comment #7) > As of yesterday, yes. And it'll stay there for 6 months or so, during which > time they are calling for people to maybe implement it. > > I await your patches most eagerly. Boris: Don't we have any reason for blocking to fix this bug? We don't have a patch for dropping -moz- prefix only?

Ah... sorry. -moz-pre-line doesn't break line by \n. I'll try to fix this.

Testcase: http://www.gtalbot.org/BugzillaSection/Bug230555WhiteSpacePreLine.html Opera 9.50a1 build 9500 and Safari 3.0.3 build 522.15.5 for Windows pass that testcase. Adding whiteboard parity tags for Opera and Safari. Note that -moz-pre-line is currently not supported and is being reported as an error in parsing value in Seamonkey 2.0a1pre rv:1.9a8pre build 2007090402.

Well, I've looked into some of how to implement this, but it's not something I can work on anytime soon, so I'll post some of my notes here. In a conversation with fantasai over IRC, I determined that CSS 3 Text is relatively stable in concept with respect to the relevant portions of white-space (actual final value names and property names are subject to change, and hyphenation falls under the category of "don't go there"). According to the current texts of CSS 3 Text, the white-space property is a shorthand property for white-space-collapsing and text-wrap properties, both of which are moderately handled under nsStyleText::WhiteSpaceIsSignificant and nsStyleText::WhiteSpaceCanWrap. The other four values of white-space represent the four boolean combinations of these two functions. The problem with pre-line is that it only certain types of white space are significant. To place pre-line within this mesh, a little more is required. The relevant place to muck around with is WhiteSpaceIsSignificant; we have two options: 1. Add a new boolean function (LineBreaksAreSignificant) that is true only for pre-line (and pre as well, I guess, but white space is significant there, so it doesn't matter too much...) 2. Make WhiteSpaceIsSignificant return an enum representing the possible values of white-space-collapsing (and change its name to something else). I personally lean to the latter since it makes transitioning to CSS 3 Text easier. A cursory examination of grep identified the following files as likely to need to be changed: editor/libeditor/base/nsEditor.cpp layout/forms/nsGfxButtonControlFrame.cpp layout/generic/nsBlockFrame.cpp layout/generic/nsFrame.cpp layout/generic/nsLineLayout.cpp layout/generic/nsTextFrameThebes.cpp layout/inspector/src/inDOMUtils.cpp Questions/Comments/Concerns ?

Here's a patch. One issue here is the interaction with text-align:justify. The spec tries to disable justification for white-space:pre and pre-line, but it doesn't make much sense. I raised the issue on www-style. For now I'm leaving our existing behaviour unchanged; pre-line text can be justified, pre-wrap can't. There's a test for this now.

Just wanted to point out that this defect also discusses pre-wrap rendering behavior... FF3 now honors rendering per CSS 2.1, but the selection copy still appears to ignore "\n". Testcase: http://jjmmjmmm.com/ff3-testcase.html

Please file that as a separate bug.

So, the IsJustificationEnabled method and its callers don't make sense to me, since you'd want to check mTextAlign on the containing block and the white-space properties on the text. Is the current code incorrectly checking mTextAlign on the text? Or is something else going on?

You're right, the current code incorrectly checks text-align on the text when it should be going out to the containing block. Would you like me to fix that as part of this patch, before this patch, or after this patch?

Any of the three is OK, but I'd prefer not to add an IsJustificationEnabled method that institutionalizes the bug in this patch. In the places where you're switching to IsJustificationEnabled, the old code was closer to correct since at least the checks were separated at the caller.

OK, this fixes the text-align behaviour (and adds a test for it of course).

Comment on attachment 328820 [details] [diff] [review] fix v2, now with tests +static const nsTextFrameUtils::CompressionMode CSSWhitespaceToCompressionMode[] = +{ + nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE, // normal + nsTextFrameUtils::COMPRESS_NONE, // pre + nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE, // nowrap + nsTextFrameUtils::COMPRESS_NONE, // pre-wrap + nsTextFrameUtils::COMPRESS_WHITESPACE // pre-line +}; Please precede this with: PR_STATIC_ASSERT(NS_STYLE_WHITESPACE_NORMAL == 0); PR_STATIC_ASSERT(NS_STYLE_WHITESPACE_PRE == 1); PR_STATIC_ASSERT(NS_STYLE_WHITESPACE_NOWRAP == 2); PR_STATIC_ASSERT(NS_STYLE_WHITESPACE_PRE_WRAP == 3); PR_STATIC_ASSERT(NS_STYLE_WHITESPACE_PRE_LINE == 4); + // This should not be used during reflow + NS_ASSERTION(!(GetStateBits() & NS_FRAME_FIRST_REFLOW), + "Can only call this on frames that have been reflowed"); Seems like you want to check NS_FRAME_IN_REFLOW here too? Or maybe NS_FRAME_IS_DIRTY (although you'd really need to check that on ancestors)? Even if you can't assert it, could you describe in the comment exactly what the rule is? -FindFirstLetterRange(const nsTextFragment* aFrag, +FindFirstLetterRange(const nsTextFragment* aFrag, const nsStyleText* aStyleText, Not needed anymore. BuildTextRunsScanner::BuildTextRunForFrames uses TEXT_JUSTIFICATION_ENABLED. Is that guaranteed to happen after it's set by nsTextFrame::Reflow? Did you check http://www.hixie.ch/tests/adhoc/css/text/white-space/mixed/001.html ? (I know we have a bug on the test being invalid or undefined... but do we still behave the same?) r+sr=dbaron

(In reply to comment #23) > + // This should not be used during reflow > + NS_ASSERTION(!(GetStateBits() & NS_FRAME_FIRST_REFLOW), > + "Can only call this on frames that have been reflowed"); > > Seems like you want to check NS_FRAME_IN_REFLOW here too? Yes. > BuildTextRunsScanner::BuildTextRunForFrames uses TEXT_JUSTIFICATION_ENABLED. > Is that guaranteed to happen after it's set by nsTextFrame::Reflow? No. That's actually a bug, I'll revise the patch to not depend on TEXT_JUSTIFICATION_ENABLED there. > Did you check > http://www.hixie.ch/tests/adhoc/css/text/white-space/mixed/001.html ? > (I know we have a bug on the test being invalid or undefined... but do > we still behave the same?) I'll check.

(In reply to comment #24) > > Did you check > > http://www.hixie.ch/tests/adhoc/css/text/white-space/mixed/001.html ? > > (I know we have a bug on the test being invalid or undefined... but do > > we still behave the same?) > > I'll check. Our behaviour on that test is unchanged (and it doesn't use pre-line).

Pushed f07d3fa9b667.

New tests justification-1 and pre-line-3 are failing on Windows. I commented them out and I'll look into them shortly.

Might be better to mark them fails-if() than commenting them out.

justification-1 fails because I assumed that white-space:pre-wrap on text that contains no newlines or double spaces would have line-breaking like white-space:normal. But that's not the case, because we can't trim spaces with pre-wrap, so when a word would fit on a line by less than a space's width, normal lets the word fit but pre-wrap doesn't.

pre-line-3 fails because of a change Simon made so that if we can't find a letter for first-letter, then we don't apply the first-letter style to any text in the frame. I fixed justification-1 by using &nbsp;<wbr> instead of spaces in the reference, to emulate pre-wrap behaviour. And I fixed pre-line-3 to check for first-letter by making the first-letter style a color change instead of a line-height change.

Pushed the test fix as changeset 078c7e6455b6.