The default bug view has changed. See this FAQ.

XPFE code assumes document.firstChild is the root element

RESOLVED FIXED

Status

SeaMonkey
General
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Jason Barnabe (np), Assigned: Ian Neal)

Tracking

(4 keywords)

unspecified
fixed-seamonkey1.0, fixed-seamonkey1.1a, fixed1.8.0.2, fixed1.8.1
Dependency tree / graph
Bug Flags:
blocking-seamonkey1.0 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

10.34 KB, patch
neil@parkwaycc.co.uk
: review+
jag (Peter Annema)
: superreview+
Scott MacGregor
: approval1.8.0.2+
Robert Kaiser
: approval-seamonkey1.0+
Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
: approval-seamonkey1.1a+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
Bug 319654 is about adding processing instructions in XUL to the DOM tree. When
that happens, any code that thinks document.firstChild is the document element
will break. We need to change it to document.documentElement.

Updated

11 years ago
Flags: blocking-seamonkey1.0?
Keywords: helpwanted

Updated

11 years ago

Comment 1

11 years ago
/editor/ /mailnews/ and /xpfe/ count as Suite for the purposes of this bug.

/extensions/irc/js/lib/command-manager.js, line 259 -- document.firstChild.appendChild (parentElem);
IRC may want its own bug filed for this.

/extensions/p3p/resources/content/p3pSummary.js, line 92 -- while (aResultDocument.firstChild &&
/extensions/p3p/resources/content/p3pSummary.js, line 93 -- aResultDocument.firstChild != docElement) {
/extensions/p3p/resources/content/p3pSummary.js, line 94 -- aResultDocument.removeChild(aResultDocument.firstChild);
This is an XML document, not a XUL document. Not part of the bug.

/extensions/venkman/resources/content/command-manager.js, line 235 -- document.firstChild.appendChild (parentElem);
Venkman may want its own bug filed for this.

/layout/html/tests/block/bugs/4324.html, line 15 -- var body=document.firstChild.childNodes.item(1);
/layout/html/tests/table/bugs/bug13526.html, line 11 -- var oList=document.firstChild.childNodes[1].childNodes[1];
These HTML tests should probably use document.body - the first one looks wrong, although the second one might work; again, layout will want its own bug filed.

/calendar/resources/content/importExport.js, line 1645 -- // xcsDocument.insertBefore(processingInstruction, xcsDocument.firstChild);
/calendar/sunbird/base/content/customizeToolbar.xul, line 66 -- document.insertBefore(newPI, document.firstChild);
Wow, someone using document.firstChild correctly ;-)

/parser/htmlparser/tests/html/18308.html, line 9 -- var oWhat=document.firstChild.childNodes[i];
Again, this code doesn't work, I guess someone needs to file a bug on parser.
(Reporter)

Updated

11 years ago
Blocks: 319654
(Reporter)

Comment 2

11 years ago
(In reply to comment #1)
> IRC may want its own bug filed for this.
Bug 319821

> This is an XML document, not a XUL document. Not part of the bug.
Plus it looks like a valid usage.

> Venkman may want its own bug filed for this.
Bug 319822

> These HTML tests should probably use document.body - the first one looks wrong,
> although the second one might work; again, layout will want its own bug filed.
Bug 319824

> Again, this code doesn't work, I guess someone needs to file a bug on parser.
Bug 319826

Comment 3

11 years ago
(In reply to comment #2)
That was quick work, two fixed already :-)
seems unlikely that Bug 319654 will end up on any branches, why do we need this for 1.0?
OS: Linux → All
Hardware: PC → All

Comment 5

11 years ago
(In reply to comment #4)
>seems unlikely that Bug 319654 will end up on any branches, why do we need this for 1.0?
Because that bug only applies to loading documents, not to mutating existing documents, which already breaks any code making the assumption.
oh, good point.
Flags: blocking-seamonkey1.0? → blocking-seamonkey1.0+
(Assignee)

Comment 7

11 years ago
*** Bug 322520 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 8

11 years ago
Created attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

Changes document.firstChild to document.documentElement
Assignee: general → iann_bugzilla
Status: NEW → ASSIGNED
Attachment #207670 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 9

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

>-      var skipBlockQuotes = (window.document.firstChild.getAttribute("windowtype") == "msgcompose");
>+      var skipBlockQuotes = (window.document.documentElement.getAttribute("windowtype") == "msgcompose");
You could probably just drop the "window" from this.

Updated

11 years ago
Attachment #207670 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

11 years ago
Attachment #207670 - Flags: superreview?(jag)

Updated

11 years ago
Attachment #207670 - Flags: superreview?(jag) → superreview+
(Assignee)

Comment 10

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

Checking in (trunk)
editor/ui/composer/content/ComposerCommands.js;
new revision: 1.203; previous revision: 1.202
editor/ui/composer/content/editorApplicationOverlay.js;
new revision: 1.22; previous revision: 1.21
mailnews/base/resources/content/mailWindowOverlay.js;
new revision: 1.229; previous revision: 1.228
mailnews/extensions/smime/resources/content/msgReadSMIMEOverlay.js;
new revision: 1.4; previous revision: 1.3
xpfe/communicator/resources/content/contentAreaDD.js;
new revision: 1.36; previous revision: 1.35
xpfe/communicator/resources/content/contentAreaUtils.js;
new revision: 1.140; previous revision: 1.139
xpfe/components/sidebar/resources/sidebarOverlay.js;
new revision: 1.127; previous revision: 1.126
xpfe/global/resources/content/charsetOverlay.js;
new revision: 1.43; previous revision: 1.42
done
Attachment #207670 - Attachment description: Simple patch v0.1 → Simple patch v0.1 (Checked in trunk)
Attachment #207670 - Attachment description: Simple patch v0.1 (Checked in trunk) → Simple patch v0.1
Attachment #207670 - Flags: approval-seamonkey1.1+
Attachment #207670 - Flags: approval-seamonkey1.0?
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: helpwanted
Resolution: --- → FIXED
(Assignee)

Comment 11

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

Checking in (branch 1.8)
editor/ui/composer/content/ComposerCommands.js;
new revision: 1.201.4.2; previous revision: 1.201.4.1
editor/ui/composer/content/editorApplicationOverlay.js;
new revision: 1.21.24.1; previous revision: 1.21
mailnews/base/resources/content/mailWindowOverlay.js;
new revision: 1.222.2.6; previous revision: 1.222.2.5
mailnews/extensions/smime/resources/content/msgReadSMIMEOverlay.js;
new revision: 1.3.126.1; previous revision: 1.3
xpfe/communicator/resources/content/contentAreaDD.js;
new revision: 1.35.2.1; previous revision: 1.35
xpfe/communicator/resources/content/contentAreaUtils.js;
new revision: 1.134.2.3; previous revision: 1.134.2.2
xpfe/components/sidebar/resources/sidebarOverlay.js;
new revision: 1.125.4.1; previous revision: 1.125
xpfe/global/resources/content/charsetOverlay.js;
new revision: 1.42.2.1; previous revision: 1.42
done
Attachment #207670 - Attachment description: Simple patch v0.1 → Simple patch v0.1 (Checked in trunk & branch 1.8)
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1
Whiteboard: fixed-seamonkey1.1

Updated

11 years ago
Attachment #207670 - Flags: approval-seamonkey1.0? → approval-seamonkey1.0+
(Assignee)

Updated

11 years ago
Blocks: 323357
(Assignee)

Comment 12

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

The two changes under editor are shared code with TB, the changes are very low risk though.
Attachment #207670 - Flags: approval1.8.0.2?

Comment 13

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

I'm ok with the overlapping changes to files shared by Thunderbird here. Since this change only impacts thunderbird and the suite, I'll take the liberty of approving this patch for the seamonkey 1.0 release on the 1.8.0.x branch.
Attachment #207670 - Flags: approval1.8.0.2? → approval1.8.0.2+
(Assignee)

Comment 14

11 years ago
Comment on attachment 207670 [details] [diff] [review]
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)

Checking in (branch 1.8.0)
editor/ui/composer/content/ComposerCommands.js;
new revision: 1.201.4.1.4.1; previous revision: 1.201.4.1
editor/ui/composer/content/editorApplicationOverlay.js;
new revision: 1.21.32.1; previous revision: 1.21
mailnews/base/resources/content/mailWindowOverlay.js;
new revision: 1.222.2.3.4.3; previous revision: 1.222.2.3.4.2
mailnews/extensions/smime/resources/content/msgReadSMIMEOverlay.js;
new revision: 1.3.134.1; previous revision: 1.3
xpfe/communicator/resources/content/contentAreaDD.js;
new revision: 1.35.10.1; previous revision: 1.35
xpfe/communicator/resources/content/contentAreaUtils.js;
new revision: 1.134.2.2.2.2; previous revision: 1.134.2.2.2.1
xpfe/components/sidebar/resources/sidebarOverlay.js;
new revision: 1.125.12.1; previous revision: 1.125
xpfe/global/resources/content/charsetOverlay.js;
new revision: 1.42.10.1; previous revision: 1.42
done
Attachment #207670 - Attachment description: Simple patch v0.1 (Checked in trunk & branch 1.8) → Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0)
(Assignee)

Comment 15

11 years ago
Closest keyword to fixed1.8.0.2 is fixed1.8.0.1
Keywords: fixed1.8.0.1
Whiteboard: fixed-seamonkey1.1 → fixed-seamonkey1.0, fixed-seamonkey1.1

Updated

11 years ago
Keywords: fixed1.8.0.1 → fixed1.8.0.2
Keywords: fixed-seamonkey1.0, fixed-seamonkey1.1
Whiteboard: fixed-seamonkey1.0, fixed-seamonkey1.1
You need to log in before you can comment on or make changes to this bug.