Non "key" node in keyset causes Mozilla to hang.

VERIFIED INVALID

Status

()

Core
Event Handling
P3
normal
VERIFIED INVALID
18 years ago
18 years ago

People

(Reporter: markh, Assigned: joki (gone))

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
A very simple XUL with a simple keyset definition causes Mozilla to hang.

Consider the attached .xul file.  Running "mozilla -
chrome "file://.../test2.xul" creates a small, blank window, as you expect.  
This works perfectly until you press any key, in which case Mozilla hangs, with 
CPU at 100%.

The problem is that the keyset is invalid.  It is missing an open "<" for the 
key element.  Thus, the Keyset ends up with a single generic "TextNode", rather 
than a Key node.

Such a simple error should not cause Mozilla to hang.

The problem is in nsXULKeyListener.cpp, in the loop starting at line 1410:
http://lxr.mozilla.org/mozilla/source/rdf/content/src/nsXULKeyListener.cpp#1410

1410   nsCOMPtr<nsIDOMNode> keyNode;
1411   aKeysetElement->GetFirstChild(getter_AddRefs(keyNode));
1412   while (keyNode) {
1413     nsCOMPtr<nsIDOMElement> keyElement(do_QueryInterface(keyNode));
1414     if (!keyElement)
1415       continue;

If the QueryInterface at 1413 fails, we attempt to ignore the item, and 
continue around the loop.  However, we have not moved to the next item in the 
iterator.  Thus, Mozilla is running hard from 1412-1415, over the exact same 
(failing) DOM item.

I have attached a patch that corrects the behaviour.
(Reporter)

Comment 1

18 years ago
Created attachment 14844 [details]
xul file that demonstrates the hang - note the missing "<" on the "key" definition.
(Reporter)

Comment 2

18 years ago
Created attachment 14845 [details] [diff] [review]
Proposed patch that prevents the hang by moving to the next node on failure.

Comment 3

18 years ago
Updating QA Contact.
QA Contact: janc → lorca
MarkH@ActiveState.com - thanks for this patch :-) However, the file you 
reference is no longer in the source tree. Could you possibly work out what 
has happened, and whether your patch is still relevant?

Thanks :-)

Gerv
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: patch
(Reporter)

Comment 5

18 years ago
This does appear to have vanished.  I can no longer repro, and found where the 
new equivilent code is (unfortunately, this is a second attempt at this note 
and I since lost it - not worth finding again) - anyway - the new code is 
implemented quite differently, and a visual check indicates it will not suffer 
the same problem.

I'm leaving all the status options alone, as I'm not sure of protocol there (or 
indeed if it will even work :-) - but feel free to close this.
Bug has been INVALIDated due to other changes in code :-) Closing...

Gerv
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → INVALID

Comment 7

18 years ago
Sounds good to me.  VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.