293654 - nested xbl elements with an id="something" on the content section asserts: attempt to remove an element that was never added: 'hep != nsnull && *hep != nsnull'

Status: NEW

(Core :: XUL, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

(you need a build with calendar installed to test this)

When you have a <datetimepicker/> in a xul dialog, and close the dialog, an
assertion beeps:
###!!! ASSERTION: attempt to remove an element that was never added: 'hep !=
nsnull && *hep != nsnull', file
/home/michiel/mozhack/tree1/mozilla/content/xul/document/src/nsElementMap.cpp,
line 281

The datetimepicker is defined at
http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/datetimepickers/datetimepickers.xml#900

steps to reproduce:
1. load the testcase (i used 'mozilla -chrome file:///tmp/test.xul')
2. close the window

(guessing xbl as component, because the datetimepicker is defined in xbl)

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: testcase

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

<dialog> and <datetimepicker> should be in XUL namespace.

The actual bug is possibly related to ID attribute handling.
<datetimepicker id="start-datetime" xmlns=""> doesn't have an ID attribute but
http://lxr.mozilla.org/seamonkey/source/content/xul/document/src/nsXULDocument.cpp#1081
tries to use the id attribute as an ID.  

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, the id/ref stuff in XULDocument is all screwed up.  It should only be
doing it for "ref" on nodes in the XUL namespace and for actual ID attributes
(as reported by GetIDAttributeName) on all elements.

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #2)
> <dialog> and <datetimepicker> should be in XUL namespace.

Yeah, i noticed that. But changing it doesn't make the assertion go away.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: workaround-like patch (checked in)

With this patch, the assertion goes away.
When poking around nsXULDocument::AddElementToMap and RemoveElementFromMap, I
found that when trying to add the timepicker to the map, there seems to be no
id, so it isn't added to the element map. When removing it, the id is
'content', and so there is an attempt to remove it from the map. Which fails,
because it was never added.
Why the id changes is a mystery to me, but removing it from the xbl makes the
problem go away.
Are there objections against this workaround? I don't see progress here on the
real problem, and when assertions go fatal, i want to be able to close
calendar's event dialog.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 193306 [details] [diff] [review]
workaround-like patch (checked in)

Checked in the workaround patch. Leaving the bug open for the real backend
problem.

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: testcase

This zip contains a testcase. There are two xbl elements, one 'containing' the
other (uses it in the content section). This extra element is needed to get the
assertion.
The first has a content with an id.
I tested with mozilla -chrome file:///`pwd`/test.xul.