Closed Bug 456273 Opened 12 years ago Closed 12 years ago

Some key events in contenteditable element crashes [@ nsNativeKeyBindings::KeyPress]

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: mats, Assigned: mats)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(3 files)

Some key events in contenteditable element crashes.

STEPS TO REPRODUCE
1. load the attached testcase
2. click on "text"
3. press CTRL+Z

ACTUAL RESULT
Crash in nightly trunk, Firefox 3 does not crash.

Patch coming up...
Flags: blocking1.9.1?
Attached file Testcase
Attached file stack
Attached patch Patch rev. 1Splinter Review
The bug is in nsXBLWindowKeyHandler::WalkHandlers() which doesn't
test the return value from nsContentUtils::DOMEventToNativeKeyEvent()
before calling the KeyPress/Up/Down methods, so 'nativeEvent' isn't
initialized properly ('nativeEvent' field has random stack value).

I took the liberty of changing the param type from nsIDOMEvent to
nsIDOMKeyEvent for DOMEventToNativeKeyEvent() since I don't think
it should be called for other types of events.

The mochitest is really a crash test but needs privileges (we can
make it a crash test when bug 448676 is fixed).
Attachment #339674 - Flags: review?(Olli.Pettay)
Depends on: 448676
Comment on attachment 339674 [details] [diff] [review]
Patch rev. 1

>+nsTextInputListener::KeyDown(nsIDOMEvent *aDOMEvent)
> {
>+  nsCOMPtr<nsIDOMKeyEvent> keyEvent(do_QueryInterface(aDOMEvent));
>+  NS_ENSURE_TRUE(keyEvent, NS_ERROR_INVALID_ARG);

>+nsTextInputListener::KeyPress(nsIDOMEvent *aDOMEvent)
> {
>+  nsCOMPtr<nsIDOMKeyEvent> keyEvent(do_QueryInterface(aDOMEvent));
>+  NS_ENSURE_TRUE(keyEvent, NS_ERROR_INVALID_ARG);


>+nsTextInputListener::KeyUp(nsIDOMEvent *aDOMEvent)
> {
>+  nsCOMPtr<nsIDOMKeyEvent> keyEvent(do_QueryInterface(aDOMEvent));
>+  NS_ENSURE_TRUE(keyEvent, NS_ERROR_INVALID_ARG);
Because this is using specialized nsIDOMKeyListener, aDOMEvent should be always
nsIDOMKeyEvent, but I think it is still better to have those checks.
Attachment #339674 - Flags: review?(Olli.Pettay) → review+
Attachment #339674 - Flags: superreview?(jonas)
Not blocking on this, but we really want this patch reviewed and landed asap!
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Priority: -- → P1
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Jonas, ping?
Attachment #339674 - Flags: superreview?(jonas) → superreview+
http://hg.mozilla.org/mozilla-central/rev/0fbeaa3dd32e

-> FIXED
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Flags: wanted1.9.0.x+ → wanted1.9.0.x?
Crash Signature: [@ nsNativeKeyBindings::KeyPress]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.