getElementsByTagName("script") doesn't work for XUL windows

RESOLVED FIXED in mozilla0.9.9

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
16 years ago
11 years ago

People

(Reporter: Robert Ginda, Assigned: Robert Ginda)

Tracking

Trunk
mozilla0.9.9
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
I'd like to present the list of included scripts in the debugger, but can't
because document.getElementsByTagName("script"); returns a NodeList of length 0.

Other tag names seem to work ok.

Updated

16 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Future
(Assignee)

Comment 1

16 years ago
Sorry dude, "Future" isn't going to work for me.  I need this by 1.0 for the
debugger.
Assignee: hyatt → rginda
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla1.0
(Assignee)

Comment 2

16 years ago
I got something working, about to post a patch.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.9
(Assignee)

Comment 3

16 years ago
Created attachment 69220 [details] [diff] [review]
proposed patch, round one

So here's what I did...
Previously, nsXULContentSink::OpenTag put *only* an nsXULPrototypeScript in the
prototype document, and these objects don't get translated into actual content.
 My changes cause ::OpenTag to insert a nsXULPrototypeElement first, *then* the
nsXULPrototypeScript.  <script>s in overlays don't actually glom on to some
element in the target document, so ::CreateOverlayElement was modified to skip
the forward reference in that case.  If we let the forward reference get
created, we end up with getElementById("") assertions when we try to resolve
it.

Text in inline scripts is not preserved in the script element.	Not perfect,
but good enough for my purposes.

Post 1.0 it might be cleaner for nsXULPrototypeScript to inherit
nsXULPrototypeElement, or just do away with nsXULPrototypeElement, and push its
members down to nsXULPrototypeNode.

This wfm on linux, I don't see any new assertions, and seem to be able to
browse just fine (oh, and getElementsByTagName("script") works too.)  I'll run
through a few more tests tomorrow.
(Assignee)

Comment 4

16 years ago
adding potential reviewers

Comment 5

16 years ago
use nshtmlatoms::script.  sr=hyatt
Comment on attachment 69220 [details] [diff] [review]
proposed patch, round one

What hyatt said, r=jst
Attachment #69220 - Flags: superreview+
Attachment #69220 - Flags: review+
(Assignee)

Comment 7

16 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

16 years ago
Oops.  That patch didn't cover top level <script> tags in overlays.  That is:
<overlay>
  <script src="file1.js"/>
  <box id="box-id">
    <script src="file2.js"/>
  </box>
</overlay>

A content element for file2.js gets created, but not file1.js, but both scripts
are executed.  I propose to add all overlay elements without IDs to the root
element in the target document.  This removes the two mNodeInfo->Equals calls
from my last patch, in favor of a single null string test, which may speed
things up a bit.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

16 years ago
Created attachment 69837 [details] [diff] [review]
more patch

These changes cause all overlay nodes without IDs to end up in the root element
of the target document.  We used to just ASSERT in this case...

###!!! ASSERTION: getElementById(""), fix caller?: '!aId.IsEmpty()', file
nsXULDocument.cpp, line 3575
Attachment #69220 - Attachment is obsolete: true
Comment on attachment 69837 [details] [diff] [review]
more patch

r/sr=brendan@mozilla.org, conditional on what the perf effects are this time
round.

/be
Attachment #69837 - Flags: superreview+
Comment on attachment 69837 [details] [diff] [review]
more patch

r=jst
Attachment #69837 - Flags: review+
(Assignee)

Comment 12

16 years ago
Checked in, I'll watch the testerboxen for possible perf issues.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 13

16 years ago
*** Bug 122138 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Depends on: 363419
You need to log in before you can comment on or make changes to this bug.