Closed Bug 233641 Opened 16 years ago Closed 15 years ago

[FIX]out-of-bounds inserts in nsXULElement::InsertChildAt on startup

Categories

(Core :: XUL, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: sicking, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Just starting the browser results in calls to nsXULElement::InsertChildAt with
indexes bigger then number of children in a node. This should not happen as such
inserts will fail (note that the index is even bigger then for an append).
Seemed to be some problem in the overlay code. Possibly an off-by-one error.
Is this when deserializing a FastLoad file?

/be
No, we deserialize into prototype nodes, which this code doesn't deal with.

It seemed to be during the process of creating and wrapping a DOM-tree around a
prototype-tree. Though of course it could be more then that. I didn't look into
why this was called once i realized i had to deal with it still.
Attached patch PatchSplinter Review
The problem here is that overlays can specify an explicit insertion index (via
an attribute).	This code needs to sanity-check the number before blindly
trying to insert at the provided index.

(Note that I suspect that overlaying non-XUL elements like this still asserts
even in today's builds, so we really do want to fix this underlying bug...)
Assignee: hyatt → bzbarsky
Status: NEW → ASSIGNED
Attachment #177631 - Flags: superreview?(bugs)
Attachment #177631 - Flags: review?(bugmail)
I'd really like to get this in for 1.8...
Blocks: 286000
Priority: -- → P1
Summary: out-of-bounds inserts in nsXULElement::InsertChildAt on startup → [FIX]out-of-bounds inserts in nsXULElement::InsertChildAt on startup
Target Milestone: --- → mozilla1.8beta2
> (Note that I suspect that overlaying non-XUL elements like this still asserts
> even in today's builds, so we really do want to fix this underlying bug...)

I don't get this comment. I don't see anything xul specific about your fix...
Comment on attachment 177631 [details] [diff] [review]
Patch

r=me given a clarification on the question in previous comment.
Attachment #177631 - Flags: review?(bugmail) → review+
> I don't get this comment. I don't see anything xul specific about your fix...

I'm removing code in nsXULElement that protected against this bug in overlay
merging.  nsGenericElement doesn't have equivalent code, so in a build without
my fix, if we overlay kids into a non-XUL node in a XUL document we'll end up
first asserting and then referencing out-of-bounds elements in the child array
of the nsGenericElement.  That's all I was saying.
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: shrir → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.