Closed
Bug 199613
Opened 22 years ago
Closed 21 years ago
We should not put textnodes as children of the documentnode
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: sicking, Assigned: sicking)
Details
Attachments
(1 file, 1 obsolete file)
|
3.57 KB,
patch
|
Details | Diff | Splinter Review |
The spec says it's forbidden
http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-1590626202
|
Assignee | |
Updated•22 years ago
|
Summary: We should not put textnodes as children for the documentnode → We should not put textnodes as children of the documentnode
|
|
Assignee | |
Comment 1•22 years ago
|
||
the change to the xul contentsink isn't strictly needed since that sink has
another test that will catch this (that test is still needed though). However i
wanted to keep the sinks in sync.
|
|
Assignee | |
Updated•22 years ago
|
Attachment #118757 -
Flags: superreview?(jst)
Attachment #118757 -
Flags: review?(peterv)
|
||
Comment 2•22 years ago
|
||
is this the only bug we have about this?
I recall saying something like "has editor approved this?". Stuff like indenting
the documentElement or comments comes to mind.
Anyway, I prefer PRUnichar('\n') over (PRUnichar)'\n'. The latter is the wrong
cast, and I would expect that the first is easier to optimize out.
(As that's really just to work around a paranoid interface is nsAString.)
|
||
Comment 3•22 years ago
|
||
Comment on attachment 118757 [details] [diff] [review]
patch to fix
- In nsDocumentEncoder.cpp:
+ if (parent) {
+ parent->GetNodeType(&type);
+ if (type == nsIDOMNode::DOCUMENT_NODE) {
+ aStr.Append((PRUnichar)'\n');
Stick to two-space indentation.
sr=jst
Attachment #118757 -
Flags: superreview?(jst) → superreview+
|
||
Comment 4•22 years ago
|
||
Comment on attachment 118757 [details] [diff] [review]
patch to fix
Please make sure the editor people are ok with this! It looks as though may
affect roundtripping.
Attachment #118757 -
Flags: review?(peterv) → review+
|
|
Assignee | |
Comment 5•22 years ago
|
||
glazou told me to run these tests:
you should try composer with you patch applied; make sure to test both modes
(see pref panel): retain original formatting vs. reformat
otherwise, the editor should be pretty agnostic about this
make sure to load AND save a document when you test
Status: NEW → ASSIGNED
|
||
Comment 6•22 years ago
|
||
this is the same as bug 56764 which had a similar patch, but was never checked
in due to my lameness and Pike's unanswered question "does this affect the
editor?". Would be nice if you could answer :-)
|
|
Assignee | |
Comment 7•21 years ago
|
||
I just tested what the trunk does and it doesn't handle any sort of content in
the root of the document, whitespace is compleatly ignored and comments are
placed in the <head> or the <body>. So I'll check this in now
|
|
Assignee | |
Comment 8•21 years ago
|
||
this is what i checked in, same as previous just synced to tip
Attachment #118757 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 9•21 years ago
|
||
upps, forgot to mark this fixed, was checked in a couple of days ago.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•