Closed Bug 50267 Opened 24 years ago Closed 24 years ago

Ill-formed browserBindings.xul crashes mozilla

Categories

(SeaMonkey :: UI Design, defect, P1)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
Windows 2000
Type:
defect

Tracking

(Not tracked)

Status:
CLOSED INVALID

People

(Reporter: timeless, Assigned: dr)

References

(
URL
)

Details

(Keywords: crash) 

<key is="emHelpKey" keycode="VK_F1" control="false" shift="false"
        onkeypress="
            window.delayedOpenWindow( getBrowserURL(), 'all,dialog=no", 
'http://www.mozilla.org/projects/seamonkey/release-notes/');
            "/>
the problem is 'all,dialog=no" [mismatched quotes].

Talkback incidents <dbaron> timeless:  16272910, 16273350, 16273362, 16273781, 
16274230 are the keybindings crash
I just pass that quoted string to JS for interpretation. This is not an
XPtoolkit bug.
I'm stumped.  Who should get it?
Sounds like JS should be flagging an error rather than crashing, but the
expedient thing to do for N6 is fix the xul file. ->xpapps.
Assignee: trudelle → don
Component: XP Toolkit/Widgets: XUL → XP Apps
QA Contact: jrgm → sairuh
Careful, if I were parsing id have:
<key
is="emHelpKey"
keycode="VK_F1"
control="false"
shift="false"
onkeypress="window.delayedOpenWindow(...=no"
, 'http://www.mozilla.org/projects/seamonkey/release-notes/');"/>

Assuming that's the case i wouldn't think it's the javascript itself that is 
messing it up, but the last line ...

Maybe parser?  It depends exactly what is messing up.
Erm i take that back (and run to dinner)
we crash when i hit the key, so the js is executing.
window.delayedOpenWindow(getBrowserURL(), 'all,dialog=no
so the js engine needs to check for matched parens or whatnot ..
sorry
timeless, what are your steps for causing this crash? thx.
Keywords: crash
There are lots of ways to crash, so i'll list them here [run moz, press f1 
from a gui, eg Profile Manager]:
<key is="emHelpKey" keycode="VK_F1" control="false" onkeypress="alert(", 
'');"/>
<key id="emHelpKey" keycode="VK_F1" onkeypress="alert(", '');"/>
<key id="emHelpKey" keycode="VK_F1" "/>
<key id="emHelpKey" keycode="VK_F1" '';/>
<key id="emHelpKey" keycode="VK_F1";/>
<key id="emHelpKey" keycode="VK_F1" a/>
<key id="emHelpKey" a keycode="VK_F1"/>
<key id="emHelpKey" a= keycode="VK_F1"/>
<key id="emHelpKey" keycode="VK_F1" a="", />
<key id="emHelpKey" keycode="VK_F1" & />
<key id="emHelpKey" keycode="VK_F1" ? />
<key id="emHelpKey" keycode="VK_F1" 1 />
<key id="emHelpKey" keycode="VK_F1" 1="2" />
<key id="emHelpKey" keycode="VK_F1" \ />

JavaScript error: line 0: syntax error
<key is="emHelpKey" keycode="VK_F1" control="false" onkeypress="alert("/>
This does not crash:
<key id="emHelpKey" keycode="VK_F1"/>
<key id="emHelpKey" a="" keycode="VK_F1"/>
<key id="emHelpKey" keycode="VK_F1" oncommand="alert('hi')" />

behaves correctly:
<key id="emHelpKey" keycode="VK_F1" onkeypress="alert('hi')" />

--
FWIW, closing the profile manager dialog (x) seemed to crash moz too. Maybe I 
should get another build :|
--
Conclusion: It isn't JavaScript's fault (my first guess just might have been 
right :o)
URL: chrome://communicator/content/platfor...
Is the stack on all of those crashes the same? What is it? 
Stack? what's a stack?
Due to a feature of QFA I crash before QFA is even loaded. Once i get on campus 
I can try using current debug builds (actually i could just use solaris, but 
I'm busy, I should be doing work instead of QA)
I'm pretty sure the stacks /would/ be the same if I  could see them...
nav traige team: [NEED INFO] Is this still reproduceable?
Keywords: nsbeta3
Whiteboard: [NEED INFO]
I see this crash on 082908 mozilla win32 talkback build on NT4.  I launch
mozilla -ProfileManager and click the X to close it and crash.  
Tested this with 082915 mozilla Mac build on OS9.  I inserted that first snippet 
into platformBrowserBindings.xul and started mozilla.  hitting F1 lcoked the app 
up completely.  had to kill the unresponsive app with command option escape.
fwiw, i wasn't able to get this crash on linux following asa's steps using
-ProfileManager.
Whiteboard: [NEED INFO]
nav triage team: assigning over to Profile Manager Frontend. We're also 
perplexed. Is it that someone is trying to add this keybinding(F1) and this has 
exposed a crash, or what?
Assignee: don → ben
Component: XP Apps → Profile Manager FrontEnd
QA Contact: sairuh → gbush
nav triage team: d'oh didn't realize ben was the owner for Profile manager 
frontend too. Anyway, we [NEED MORE INFO]. How does the keybinding(F1) issue 
relate to crashing the profile manager when clicking the close box (which we 
were able to repro)? Is this two separate bugs?
Whiteboard: [NEED INFO]
close box bug is bug 49615
This bug was just misfiled. F1 will crash at any point. I just use profile 
manager because it loads fast.

The crashing by clicking the close bug is just another bug. I'm sorry I 
mentioned it.
So basically, this is a 'make xul/xml parser more robust' bug?
Not necessarily make it "smarter" (we've seen html, not again thanks), just make 
it not crash when encountering bad/incorrect/corrupted xul/xml?
nav triage team:
assuming everytime you click F1 on Windows 2000 causes a crash, this is a
nsbeta3+ P1.

Changing component to XP Apps.
Component: Profile Manager FrontEnd → XP Apps
Priority: P3 → P1
QA Contact: gbush → sairuh
Whiteboard: [NEED INFO] → [NEED INFO][nsbeta3+]
I cannot find the code displayed in the file that this refers to, nor reproduce
this crash as no steps were supplied.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → WORKSFORME
Ben. pick any of the strings from  timeless@bemail.org 2000-08-26 22:46
copy it into the browserBindings file.  Run mozilla and press F1.  I think this 
belongs to hyatt.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
so this is nothing to do with anything in the tree? 

removing nsbeta3+ then.
Whiteboard: [NEED INFO][nsbeta3+] → [NEED INFO]
nav triage team:
Claudius is creating a new bug for F1 causes crash, and we will + that one.
This one gets a minus.
Whiteboard: [NEED INFO] → [nsbeta3-]
F1 Help crasher bug 51908
Invalid in the context of XML means "not conforming to DTD." Ill-formed, meaning
"not conforming to XML syntax" is what you mean. Mismatching quotes causes
ill-formedness. The XML parser should not allow ill-formedness, as that would
make our XML parser non-conformant :)

In other words, just fix the damn quotes. And if this isn't even in the tree,
then why is this bug still open?
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → INVALID
Summary: Invalid browserBindings.xul crashes mozilla → Ill-formed browserBindings.xul
Well, this could just be morphed into "[RFE] Make Mozilla not crash on 
ill-formed xul/javascript" :-)
QA Contact: sairuh → jrgm
jag: you don't want that. trust me. you just really don't want that.

everybody and their brother write ill-formed, downright nasty html. this is only
encouraged by the fact that all browsers accept it. the same situation should
not occur with xml -- xml parsers are very easy to use: just as html authors
test their pages in a browser, xml authors can test their stuff in mozilla or
whatever other new generation browser they use...

also, we do not crash on ill-formed xml. we crash on F1 on win2k (bug 51908).

the short story is: xul is xml. xml is supposed to be well-formed. parsers that
accept ill-formed xml are non-conformant. we are conformant. write correct xul.
dr: From what I understood from talks with timeless, mozilla _was_ crashing on
invalid xul, the actual key used didn't matter. I may have misunderstood that
though.

Anyway, I agree with you on invalid/ill-formed xml, so I think you're
misunderstanding me here.

If mozilla encounters invalid or ill-formed XML, it should make the user aware
of that, perhaps through an error message, perhaps by just not doing anything,
I'll let others think up something clever on that. If it's invalid XUL in the
chrome, it's even more important the developer is made aware of the error.
Mozilla should _never_ try to guess what was meant. But, it should _not_ crash.
Though, that is a good way to get someone's attention and get the XUL fixed ;-)

Catch me in irc if things are still unclear.
jag: ah, now i understand. the value of clarity is high...

anyway, feel free to open a new bug re: ill-formed xul crashing mozilla. i'll
bet it's just a case of a component not anticipating a strange error case (like,
the xml parser cleanly handling the error, but the xul impl not understanding
it...) just double-check to see if it's still a problem (by breaking a xul file
or something).

somebody care to verify this bug's resolution?
Marking verified invalid, if for no other reason than it would be more 
effective to file new bugs for specific cases where where crash on "bad XUL",
instead of morphing this bug for that purpose.

Clearly, it is preferable to flag an error, and abort loading when loading
XUL that is either (1) not well-formed XUL/XML, or (2) well-formed, but invalid
XUL (e.g., elements placed where they should not be placed).
Status: RESOLVED → VERIFIED
Thank you for trashing this bug</sarcasm> Please file all of the relevant bugs 
and CC me.

Your resolution is unfair.  I filed this bug w/ the best summary I could, I 
don't speak all dialects of technobabel and am sorry I didn't know the 
difference between invalid and ill formed.

The recommendation you are making is to file more bugs w/ the same summary.

Removing keywords, status. Marking meta.

Would someone please attach the stack traces from the original report?  
Commentary about what's happening according to the stack traces would be 
useful.
Status: VERIFIED → REOPENED
Keywords: nsbeta3 → meta
Resolution: INVALID → ---
Summary: Ill-formed browserBindings.xul → Ill-formed browserBindings.xul crashes mozilla
Whiteboard: [nsbeta3-]
timeless: please re-close this bug as invalid, or open a new bug for ill-formed
xul crashing mozilla, and close it as a dup of that. this bug was originally
misfiled and therefore has too much misinformation to be clear for use in fixing
the bug you *really* want fixed. this is what jrgm and i were talking about, and
has nothing to do with technobabble.

i'm taking this bug myself to make sure you resolve it appropriately.
Assignee: ben → dr
Status: REOPENED → NEW
Keywords: meta
Status: NEW → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → INVALID
A simple test of mangling navigator.xul results not in a crash, but in:

MOZILLA_FIVE_HOME=/home/dr/mozilla/dist/bin
  LD_LIBRARY_PATH=/home/dr/mozilla/dist/bin
    
LIBRARY_PATH=/home/dr/mozilla/dist/bin:/home/dr/mozilla/dist/bin/components      
SHLIB_PATH=/home/dr/mozilla/dist/bin
          LIBPATH=/home/dr/mozilla/dist/bin
       ADDON_PATH=/home/dr/mozilla/dist/bin
      MOZ_PROGRAM=/home/dr/mozilla/dist/bin/mozilla-bin
      MOZ_TOOLKIT=
        moz_debug=0
     moz_debugger=
WEBSHELL+ = 1
WEBSHELL+ = 2
XML Error in file 'chrome://navigator/content/navigator.xul', Line Number: 40,
Col Number: 11, Description: not well-formed

What this shows is that the XML parser does indeed inform the XUL implementation
of parse errors, and that mozilla does not crash, but closes quietly. This bug
is INVALID. If you can find a different case (mangling other XUL files in the
tree) that crashes mozilla, /then/ open a new bug and post the stack trace.

Timeless: in other words, this bug was not marked invalid because I didn't
understand your vagueness or because it was misfiled, but because it was
invalid, and nothing more.

Please be aware that lack of knowledge ("stack? what's a stack?" and "i don't
speak all dialects of technobabble") is no excuse for rudeness, and being
flat-out wrong (this isn't a crasher) doesn't help either. I have a lot of
respect for your seemingly 24x7 commitment to the mozilla project (you spend
more time than I do, and you don't get paid to do this!) but you can't take a
high-and-mighty attitude, or expect that sort of respect, because of your
commitment alone.

Please contact me privately if you feel I'm being unfair, and keep in mind that
it's easy to let your frustration stand in the way of the very same thing you so
strongly commit yourself to.
I'm closing this bug. I know we very much object to closed bugs, but this one 
shouldn't be verified, and i don't want it verified, i want it closed.

Bug 52359 will cause this problem to go away.

This bug was a feature of the loading system we used for the 
(platform)[Browser|Editor]Bindings.xul files.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/rdf/content/src/nsXULKeyLis
tener.cpp&rev=1.77
1253 nsCOMPtr<nsIXULContentSink> sink;
1278 parser->SetContentSink(sink);
1284 rv = parser->Parse(aURI);
1285 if (NS_FAILED(rv)) return rv;
I'm guessing 1284 returns NS_FAILED and that 1285 returns while the sink is 
still in some unhappy state.  But I don't understand nsCOMPtr<>.
Status: RESOLVED → CLOSED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.