Closed
Bug 814677
Opened 13 years ago
Closed 13 years ago
For the purpose of table frame fixup, "form feed" (U+000C) should also be considered a white space.
Categories
(Core :: Layout: Tables, defect)
Core
Layout: Tables
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: poying.me, Assigned: MatsPalmgren_bugz)
Details
(Keywords: css2, testcase)
Attachments
(1 file)
|
9.39 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121120063346
Steps to reproduce:
Open the
data:text/html,<style> span { display: table-cell; } </style><span>A</span>%0C<span>B</span>
Actual results:
display
A
B
Expected results:
Should display
AB
Component: Untriaged → Style System (CSS)
Product: Firefox → Core
Version: 17 Branch → Trunk
|
||
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Component: Style System (CSS) → Layout: Tables
Ever confirmed: true
Summary: Only the characters "space" (U+0020), "tab" (U+0009), "line feed" (U+000A), "carriage return" (U+000D), and "form feed" (U+000C) can occur in white space. → For the purpose of table frame fixup, "form feed" (U+000C) should also be considered a white space.
Poying
It seems like we need to add "form feed" (U+000C) in NS_IsSpace
http://hg.mozilla.org/mozilla-central/annotate/e685ffb06975/intl/lwbrk/public/nsILineBreaker.h#l57
static inline bool
NS_IsSpace(PRUnichar u)
{
return u == 0x0020 || // SPACE
u == 0x0009 || // CHARACTER TABULATION
+ u == 0x000C || // CARRIAGE RETURN
u == 0x000D || // FORM FEED
(0x2000 <= u && u <= 0x2006) || // EN QUAD, EM QUAD, EN SPACE,
// EM SPACE, THREE-PER-EM SPACE,
// FOUR-PER-SPACE, SIX-PER-EM SPACE,
(0x2008 <= u && u <= 0x200B); // PUNCTUATION SPACE, THIN SPACE,
// HAIR SPACE, ZERO WIDTH SPACE
}
And it is correct in http://dxr.mozilla.org/mozilla-central/content/base/src/nsContentUtils.cpp.html#l1196
|
||
Comment 2•13 years ago
|
||
Or leave NS_IsSpace as-is and change the function frame construction calls here?
|
Assignee | |
Comment 3•13 years ago
|
||
I took a stab at fixing this. I think some of the consumers should probably
take the content language into account though. Also, this is the third
implementation (at least) that tests for these five space characters.
There's also nsContentUtils::IsHTMLWhitespace and IsWhitespace in
nsCSSScanner.cpp. While having separate DOM/HTML/CSS names makes the code
clear, they could probably share a common (fast) implementation.
(The CSS one looks like the fastest, but it can be further optimized)
That's for future bugs though.
Boris, let me know if you want XXX comments in any of the changed
places that it should take the content language into account.
I chose the name dom::IsSpaceCharacter to try to associate it with
the term "space character" in the DOM spec.
http://dom.spec.whatwg.org/#space-character
Assignee: poying.me → matspal
Attachment #685900 -
Flags: review?(bzbarsky)
|
|
||
Comment 4•13 years ago
|
||
Comment on attachment 685900 [details] [diff] [review]
fix + test
r=me. FIXME comments and a followup bug to combine things might not be a bad idea.
Attachment #685900 -
Flags: review?(bzbarsky) → review+
|
|
Assignee | |
Comment 5•13 years ago
|
||
OK, I'll add a few FIXMEs where I think it makes sense to take language into
account when determining what is whitespace. Meanwhile, a related issue,
what should the result be for:
data:text/html,<style> span { display: table-cell; } body {white-space:pre} </style><body><span>A</span> <span>B</span>
Firefox and IE10 renders:
AB
Opera and Chrome renders:
A
B
Should the frame constructor take the 'white-space' property into account
when determining "ignorable whitespace"?
|
|
||
Comment 6•13 years ago
|
||
(In reply to Mats Palmgren [:mats] from comment #5)
> Meanwhile, a related issue, what should the result be for:
> data:text/html,<style> span { display: table-cell; } body {white-space:pre}
> </style><body><span>A</span> <span>B</span>
I think CSS 2.1 17.2.1 has taken 'white-space: pre' into account in response to Boris message[1] so Firefox and IE10 are probably correct. I couldn't find a test like this in [2] however.
[1] http://lists.w3.org/Archives/Public/www-style/2009May/0213
[2] http://test.csswg.org/harness/results/CSS21_DEV/section/17.2.1/
|
|
Assignee | |
Comment 7•13 years ago
|
||
Thanks Kenny, that answers my question.
|
|
||
Comment 8•13 years ago
|
||
The spec on this was underdefined for a while, but I believe at this point Gecko and IE's behavior is what's specced. In particular, see the definition of "consecutive" at http://www.w3.org/TR/CSS21/tables.html#anonymous-boxes which just talks about "whitespace", no matter what the styling is.
|
|
Assignee | |
Comment 9•13 years ago
|
||
Added a few FIXMEs, and added more tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad838a319df
|
|
Assignee | |
Comment 10•13 years ago
|
||
(filed bug 817004 to follow-up on comment 3)
|
||
Comment 11•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•