Last Comment Bug 319659 - XPFE code assumes document.firstChild is the root element
: XPFE code assumes document.firstChild is the root element
Status: RESOLVED FIXED
: fixed-seamonkey1.0, fixed-seamonkey1.1a, fixed1.8.0.2, fixed1.8.1
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Ian Neal
:
:
Mentors:
http://lxr.mozilla.org/seamonkey/sear...
: 322520 (view as bug list)
Depends on:
Blocks: 319654 323357
  Show dependency treegraph
 
Reported: 2005-12-08 22:11 PST by Jason Barnabe (np)
Modified: 2006-01-26 15:44 PST (History)
5 users (show)
cbiesinger: blocking‑seamonkey1.0+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Simple patch v0.1 (Checked in trunk & branch 1.8/1.8.0) (10.34 KB, patch)
2006-01-05 15:23 PST, Ian Neal
neil: review+
jag-mozilla: superreview+
mscott: approval1.8.0.2+
kairo: approval‑seamonkey1.0+
csthomas: approval‑seamonkey1.1a+
Details | Diff | Splinter Review

Description Jason Barnabe (np) 2005-12-08 22:11:29 PST
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.
Comment 1 neil@parkwaycc.co.uk 2005-12-09 02:44:25 PST
/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.
Comment 2 Jason Barnabe (np) 2005-12-10 15:59:12 PST
(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 neil@parkwaycc.co.uk 2005-12-11 09:19:10 PST
(In reply to comment #2)
That was quick work, two fixed already :-)
Comment 4 Christian :Biesinger (don't email me, ping me on IRC) 2005-12-15 03:30:12 PST
seems unlikely that Bug 319654 will end up on any branches, why do we need this for 1.0?
Comment 5 neil@parkwaycc.co.uk 2005-12-15 09:52:07 PST
(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.
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2005-12-15 14:16:35 PST
oh, good point.
Comment 7 Ian Neal 2006-01-05 14:37:40 PST
*** Bug 322520 has been marked as a duplicate of this bug. ***
Comment 8 Ian Neal 2006-01-05 15:23:59 PST
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
Comment 9 Jason Barnabe (np) 2006-01-05 16:13:26 PST
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.
Comment 10 Ian Neal 2006-01-06 12:39:40 PST
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
Comment 11 Ian Neal 2006-01-06 12:47:56 PST
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
Comment 12 Ian Neal 2006-01-20 15:20:08 PST
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.
Comment 13 Scott MacGregor 2006-01-23 15:10:55 PST
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.
Comment 14 Ian Neal 2006-01-23 15:38:31 PST
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
Comment 15 Ian Neal 2006-01-23 15:39:43 PST
Closest keyword to fixed1.8.0.2 is fixed1.8.0.1

Note You need to log in before you can comment on or make changes to this bug.