Uncaught exception: [Exception..."Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMEventTarget.addEventListener)

RESOLVED FIXED in Firefox 7

Status

()

Core
DOM: Events
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: aaronmt, Assigned: sicking)

Tracking

({dev-doc-complete, regression, testcase})

Trunk
mozilla9
ARM
Android
dev-doc-complete, regression, testcase
Points:
---

Firefox Tracking Flags

(firefox7+ fixed, firefox8+ fixed)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110627 Firefox/7.0a1 Fennec/7.0a1
Device: Nexus One, Nexus S
OS: Android 2.3.4

I get this upon visiting the URL above.

uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMEventTarget.addEventListener]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: http://cdn.nearbyad.com/javascripts/MBS_mobile.js?426079 :: <TOP_LEVEL> :: line 352"  data: no]
Created attachment 543154 [details]
testcase

And addEventListener for 'resize' seems to be causing this for Fennec on the Android device. I'm not seeing it on desktop windows Fennec.
Created attachment 543157 [details]
testcase

*sigh*, That teaches me not to test. Apparently, when the function is defined after the listener, this error will appear on Fennec Android.
Attachment #543154 - Attachment is obsolete: true
Is this an IPC bug or something?

Updated

6 years ago
Keywords: testcase

Comment 4

6 years ago
According to that JS link, the offending bit is:

var supportsOrientationChange = "onorientationchange" in window,
orientationEvent = supportsOrientationChange ? "orientationchange" : "resize";

window.addEventListener(orientationEvent, MBS.mbsCenterAd, false);

...which I see you figured out. As well as the reason the error occurs. Ok, I'm following now.

Comment 5

6 years ago
Given that I see the same error on Firefox with your testcase, this doesn't seem to be anything isolated to IPC behaviour.
Ah, yes. I can see the issue in today's Firefox trunk, but not on Firefox 5, so this looks like a regression.
Component: General → DOM: Events
Keywords: regression, regressionwindow-wanted
Product: Fennec → Core
QA Contact: general → events
In the attached testcase, x is undefined at the point of the addEventListener call, so I would expect it to throw...
On the other hand, looks like other UAs do not.

Is this happening on aurora too?
tracking-firefox7: --- → ?
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=48e72227c2fa&tochange=e0b805cab438

So not an aurora problem.
Blocks: 658714
So it looks like nsGlobalWindow::AddEventListener used to directly call nsEventListenerManager::AddEventListenerByType.  This last does NOT null-check aListener, but used to call the internal nsEventListener::AddEventListener and ignore the return value, so always returned NS_OK

Now we're returning the return value of nsEventListenerManager::AddEventListener, which is NS_ERROR_FAILURE in this case.

We need to eat this exception somewhere.  I suggest making nsEventListenerManager::AddEventListener just be a no-op when passed null...
Keywords: regressionwindow-wanted
Unless this breaks more sites I think we should evangelize here instead.
AddEventListener must not throw. This is just a bug to fix.

Updated

6 years ago
tracking-firefox7: ? → +
(In reply to comment #11)
> Unless this breaks more sites I think we should evangelize here instead.

(In reply to comment #12)
> AddEventListener must not throw. This is just a bug to fix.

This is tracking firefox 7 - what approach do we want to take here for the FF7 release?
Fix the bug.
Assignee: nobody → jonas
Created attachment 551529 [details] [diff] [review]
Patch to fix
Attachment #543157 - Attachment is obsolete: true
Attachment #551529 - Flags: review?(Olli.Pettay)
Comment on attachment 551529 [details] [diff] [review]
Patch to fix

>+<script class="testbody" type="text/javascript">
>+
>+xhr = new XMLHttpRequest;
>+w = new Worker("empty.js");
>+window.addEventListener("load", null, false);
>+document.addEventListener("load", null, false);
>+document.body.addEventListener("load", null, false);
>+xhr.addEventListener("load", null, false);
>+w.addEventListener("load", null, false);
>+window.addEventListener("load", undefined, false);
>+document.addEventListener("load", undefined, false);
>+document.body.addEventListener("load", undefined, false);
>+xhr.addEventListener("load", undefined, false);
>+w.addEventListener("load", undefined, false);
>+
>+</script>


Shouldn't you have ok(true, "Test passed");
right before </script>

With that, r=me
Attachment #551529 - Flags: review?(Olli.Pettay) → review+
Created attachment 551558 [details] [diff] [review]
Trunk patch

This also makes addEventListener not return an nsresult since on trunk aListener being null was the only "real" way addEventListener could fail.
Attachment #551558 - Flags: review?(Olli.Pettay)
Comment on attachment 551529 [details] [diff] [review]
Patch to fix

This fixes a regression where functions that used to be no-ops were caused to throw. The patch reverts behavior so that they are a no-op again.
Attachment #551529 - Flags: approval-mozilla-aurora?
Comment on attachment 551529 [details] [diff] [review]
Patch to fix

Get it in before beta next monday, please!
Attachment #551529 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Fixed on aurora:

http://hg.mozilla.org/releases/mozilla-aurora/rev/54aa371f173b
status-firefox7: --- → fixed
Backed out because the test is failing on Aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/e835bb5bb615

http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Aurora/1312929770.1312931139.1570.gz
45037 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug667612.html | [SimpleTest/SimpleTest.js, window.onerror] - An error occurred: uncaught exception: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIWorker.addEventListener]" nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: http://mochi.test:8888/tests/content/events/test/test_bug667612.html :: <TOP_LEVEL> :: line 27" data: no] at :0
status-firefox7: fixed → affected
Turns out workers go through completely different codepaths on the aurora branch, which never worked. It does appear to work on trunk though where we're using a totally new worker implementation.

Repushed to branch with the offending tests removed:
http://hg.mozilla.org/releases/mozilla-aurora/rev/c2f69b271358
status-firefox7: affected → fixed
We couldn't decide whether or not we should transplant anything on Aurora after the migration today, so marking as tracking-firefox8+.  Jonas, can you please let us know if we need to do anything on Aurora?
tracking-firefox8: --- → +
Comment on attachment 551529 [details] [diff] [review]
Patch to fix

Good catch! Since the other patch is still is awaiting review on trunk, we'll have to take this one on (the new, to-be-ff-8) aurora.
Attachment #551529 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?

Updated

6 years ago
Attachment #551529 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Checked in to aurora:

http://hg.mozilla.org/releases/mozilla-aurora/rev/246acc814f72
status-firefox8: --- → fixed

Updated

6 years ago
Attachment #551558 - Flags: review?(Olli.Pettay) → review+
Checked in to inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/63b8853b1d67

Thanks for the review!
http://hg.mozilla.org/mozilla-central/rev/63b8853b1d67
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Keywords: dev-doc-needed
I take it all that I'm documenting here is that addEventListnener() now returns as if nothing's wrong if the listener is null?
Yup
Documentation updated:

https://developer.mozilla.org/en/DOM/element.addEventListener#Gecko_notes
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMEventTarget

And mentioned on Firefox 9 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.