element.builder.rebuild() crashes in builds 20040309 and later

Status

()

Core
DOM
--
critical
13 years ago
5 years ago

People

(Reporter: HJ, Assigned: Christopher Aillon (sabbatical, not receiving bugmail))

Tracking

({crash, testcase})

Trunk
x86
Windows XP
crash, testcase
Points:
---
Bug Flags:
blocking1.7 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
Caillon asked me (http://bugzilla.mozilla.org/show_bug.cgi?id=236946#c7) to file
a new bug for this so I did. 

I keep crashing with element.builder.rebuild() It still works in 20040308 but
fails in all builds with the patch for bug 236946 (cleanup in nsXULElement).
Here's a small code example:

          <menu label="&qpHTTPUserAgentSettings.label;">
            <menupopup id="UAgentMenu" datasources="rdf:null"
ref="urn:useragent-data" 
                       onpopupshowing="initUserAgentMenu(this);">
              <template>
                <rule>
                  <menuitem uri="rdf:*" label="rdf:*" type="radio"/>
                </rule>
              </template>
            </menupopup>
          </menu>

function initUserAgentMenu(aParent)
{
  aParent.builder.rebuild(); <= crash !!!
(Reporter)

Comment 1

13 years ago
Steps to reprodue are:

1) install MultiZilla
2) click on QPrefs button
3) hover over 'Browser Spoofing'

Current result : crash
Expected result: no crash

I think this is caused by this change:

-                    return
builder->CreateContents(NS_STATIC_CAST(nsIStyledContent*, unconstThis));
+                    return builder->CreateContents(unconstThis);

Raj, can you please add dr watson/talkback data if they need it, you probably
know that Neil and I can't (read are not allowed to do this kind of actions).

Thanks,
/HJ

Comment 2

13 years ago
Mozilla crashes silently on my WinXP build (2004-03-11-08-trunk) and although it
creates a Dr Watson log in my Win2K build (2004-03-21-08-trunk), I don't think
that Talkbalk was installed (as in, not an option, not that I chose not to
install it).

Comment 3

13 years ago
Created attachment 144643 [details]
Dr Watson log

In case it's any use, I've attached the Dr Watson stack dump.  As I said in
comment 2, Talkback was not part of my build, so I can't provide an ID

Comment 4

13 years ago
Someone from the MozillaZine forums reproduced the MultiZilla crash and reported
talkback ID TB4559G captured at: 3/24/2004 4:33 PM
(Reporter)

Comment 5

13 years ago
Christopher, I'm reassigning this bug to you, because we think that it was your
patch that caused this crasher bug. Can you please fix this bug, or at least
backout the lines that caused this bug?
Assignee: general → caillon
Severity: normal → major
(Reporter)

Comment 6

13 years ago
-> CrashWeek Effort
Severity: major → critical
Summary: element.builder.rebuild() fails in builds 20040309 and later → element.builder.rebuild() crashes in builds 20040309 and later

Comment 7

13 years ago
I just used HJ's bugzilla account because of this message: 

" You tried to change the Severity field from major to critical, but only the
owner or submitter of the bug, or a sufficiently empowered user, may change that
field."
(Reporter)

Comment 8

13 years ago
mozilla 20040309 still crashes for me on WinXP. I will add a testcase that
generated a file called: "bug-238413.rdf" in your profile directory. The content
of that file looks like this:

<?xml version="1.0"?>
<RDF:RDF xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:MZ="http://multizilla.mozdev.org/rdf#">
<RDF:Seq RDF:about="urn:useragent-data">
<RDF:li>Line one</RDF:li>
<RDF:li>Line two</RDF:li>
<RDF:li>Line three</RDF:li>
<RDF:li>Line four</RDF:li>
<RDF:li>Line five</RDF:li>
</RDF:Seq>
</RDF:RDF>

Loading the XUL file (from chrome) should display: "start, adding datasource,
rebuilding and done" but crashes in 20040308+ builds.
(Reporter)

Comment 9

13 years ago
Created attachment 145360 [details]
xul text case (can only be loaded from chrome)
(Reporter)

Comment 10

13 years ago
Christopher, anything else that we can do to help?
Keywords: testcase

Updated

13 years ago
Keywords: crash

Updated

13 years ago
Flags: blocking1.7?
(Reporter)

Comment 11

13 years ago
Jan, can you please have a look at this? This should really be fixed for mozilla
1.7 final. We really need this for work. We're stuck with old mozilla builds
now, because of a serious security issues in prior builds (not this bug). It
would be very nice if you could gear this up a bit, thanks.

Comment 12

13 years ago
From bug 236946:

Index: content/xul/content/src/nsXULElement.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/xul/content/src/nsXULElement.cpp,v
retrieving revision 1.512
diff -p -u -d -8 -r1.512 nsXULElement.cpp
--- content/xul/content/src/nsXULElement.cpp	6 Mar 2004 00:22:02 -0000	1.512
+++ content/xul/content/src/nsXULElement.cpp	9 Mar 2004 21:18:18 -0000

// XXX hack because we can't use "mutable"
nsXULElement* unconstThis = NS_CONST_CAST(nsXULElement*, this);

nsCOMPtr<nsIXULTemplateBuilder> builder;

- return builder->CreateContents(NS_STATIC_CAST(nsIStyledContent*, unconstThis));
+ return builder->CreateContents(unconstThis);

from nsIXULTemplateBuilder.idl:
[ptr] native nsIContent_ptr(nsIContent);
[noscript] void createContents(in nsIContent_ptr aElement);
Is is safe to pass |uncostThis| there?
(Reporter)

Comment 13

13 years ago
Hmm, InsertElementAt() also fails in these builds but RemoveElementAt() still works.

note: GoogleBox now also crashes as a result of this bug :(
It should be just as safe to pass nsXULElement* as it is to pass
nsIStyledContent* to a method which wants nsIContent* since both are unambiguous
descendants of nsIContent*.  I really don't understand why that change would
cause this to break (if it really does).

HJ, a testcase that I can load from non-chrome would be a bigger help.
(Reporter)

Comment 15

13 years ago
(In reply to comment #14)
> ... I really don't understand why that change would cause this to break (if it
really does).

Well, I can't find anything else in bonsai for this time time frame, do you?

> HJ, a testcase that I can load from non-chrome would be a bigger help.

Eh, and how should that work? I get a bunch of (security) errors when I try.

All you have to do is to add the attachment to your comm.jar and use
chrome://navigator/content/filename.xul You can simply remove it when you're
done testing. That should do the trick.

Btw, what about comment #4? Is that talkback data any good? 
(In reply to comment #14)
> It should be just as safe to pass nsXULElement* as it is to pass
> nsIStyledContent* to a method which wants nsIContent* since both are unambiguous
> descendants of nsIContent*.  I really don't understand why that change would

Well, that's theoretically true, but we frequently-enough key hashtables on
nsIContent*, and you would get a different pointer depending on how you do the cast.

Comment 17

13 years ago
Does this impact the SeaMonkey (or Firefox or Thunderbird) applications?

Updated

13 years ago
Flags: blocking1.7? → blocking1.7-
(Reporter)

Comment 18

13 years ago
(In reply to comment #17)
> Does this impact the SeaMonkey (or Firefox or Thunderbird) applications?

Yes and no. Yes, if you add extensions like MultiZilla,but there might be others
having the same kind of difficulties.

Asa, this is one of the issues that keeps us from having a MultiZilla version
for Mozilla Firefox, so yes, it would be nice if this could be fixed for Mozilla 1.8
HJ, could you revert the part of the patch Jan mentioned in comment 12 and let
me know if that helps?
(Reporter)

Comment 20

13 years ago
(In reply to comment #19)
> HJ, could you revert the part of the patch Jan mentioned in comment 12 and let
> me know if that helps?

No I can't, I wich I could. Also, it still crashes like before.
(Reporter)

Comment 21

13 years ago
s/wich/wish

Note to self: open your eyes, for real, and have breakfast first, before adding
comments at 6:45AM ;)
(Reporter)

Comment 22

12 years ago
Can someone please make that change so that I can test it?
QA Contact: ian → general
probably worth retesting. 
Note HJ's email address is dead.
You need to log in before you can comment on or make changes to this bug.