Closed Bug 373268 Opened 18 years ago Closed 18 years ago

page break characters, as code points, do not collapse

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: ray, Assigned: roc)

References

Details

Attachments

(1 file, 4 obsolete files)

new reftests, patch is in diff -uNp8 format
(deleted), patch
roc
: review+
roc
: superreview+
Details | Diff | Splinter Review
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a3pre) Gecko/20070304 Minefield/3.0a3pre I created two html files to use as a reftest: test.html and a reference.html: test.html: <html><head><title>Single-Space</title></head> <body> XXX&#x000C;&#x000C;&#x000C;&#x000C;&#x000C;XXX<br> </body></html> reference.html: <html><head><title>Single-Space</title></head> <body> XXX XXX </body></html> These are supposed to come out exactly the same and they do not. A single &#x000C; does get displayed as the same as an ascii space. But obviously a sequence of page break characters do not collapse to a single space.
Attached file a reftest reference page (obsolete) (deleted) —
Attached file a reftest test page (obsolete) (deleted) —
I think this is a bug in nsScanner::SkipWhitespace(). Reference: http://www.w3.org/TR/html401/struct/text.html#didx-white_space-1
Assignee: nobody → mrbkap
Component: Layout → HTML: Parser
OS: Mac OS X → All
QA Contact: layout → parser
Hardware: PC → All
So, while nsScanner::SkipWhitespace does need to take form feeds into account, it seems that layout needs to deal with collapsing the whitespace itself -- the parser doesn't do any of that (or else switching white-space: pre on and off wouldn't work correctly).
Yeah, that's my bug. You can make this a layout bug and assign it to me or file a new bug for me.
BTW is there a spec somewhere that says that FFs should be considered CSS whitespace?
Assignee: mrbkap → nobody
Component: HTML: Parser → Layout
QA Contact: parser → layout
Assignee: nobody → roc
Oh silly me. Never mind.
Attached file consolidated reftests for space collapsing issues (obsolete) (deleted) —
FYI, I am looking at things on the trunk build. The behavior has changed. Now, it appears that a &#x000C; is rendered as a "missing character", a box. I have also created a reftest for the single pagebreak (&#x000C;) case.
Attachment #257901 - Attachment is obsolete: true
Attachment #257902 - Attachment is obsolete: true
Attachment #259455 - Flags: review?(roc)
Why is this file zipped? Can you attach a patch in CVS diff format?
BTW my new-textframe code is supposed to fix this.
Depends on: 333659
I do get how the reftests are being placed into the source directories. Into which directory do you want these landed? Or does that matter?
Attached patch patch for files previously attached as tarball (obsolete) (deleted) — — Splinter Review
roc: as you requested. FYI, I did the diff with: diff -x CVS -e -r -N ../newer/mozilla/layout/reftests mozilla/layout/reftests
Attachment #259455 - Attachment is obsolete: true
Attachment #259486 - Flags: review?(roc)
Attachment #259455 - Flags: review?(roc)
CSS has very specific instructions for whitespace collapsing: http://www.w3.org/TR/CSS21/text.html#white-space-prop and they don't mention form feed characters. I'm inclined to follow CSS here and ignore the HTML spec.
So I'm afraid to say that this is, after all, INVALID.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → INVALID
Perhaps a test that asserts this behavior holds is in order, since deciding upon the test's invalidity took more than a trivial amount of effort? Should just be changing == to != in the list, so a new patch probably isn't needed.
Flags: in-testsuite?
(Bonus points for adding a comment to the tests explaining why the testcase is valid and that it respects the CSS but not HTML specs, preferably with URL references.)
Comment on attachment 259486 [details] [diff] [review] patch for files previously attached as tarball I'd like a new patch, in unified diff format, please
Attachment #259486 - Flags: review?(roc)
As requested, in better format.
Attachment #259486 - Attachment is obsolete: true
Attachment #281368 - Flags: review?(roc)
Comment on attachment 281368 [details] [diff] [review] new reftests, patch is in diff -uNp8 format good
Attachment #281368 - Flags: superreview+
Attachment #281368 - Flags: review?(roc)
Attachment #281368 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: