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.
Is this an IPC bug or something?
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.
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.
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?
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=48e72227c2fa&tochange=e0b805cab438 So not an aurora problem.
6 years ago
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...
6 years ago
Unless this breaks more sites I think we should evangelize here instead.
AddEventListener must not throw. This is just a bug to fix.
(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.
Created attachment 551529 [details] [diff] [review] Patch to fix
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.
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.
Comment on attachment 551529 [details] [diff] [review] Patch to fix Get it in before beta next monday, please!
Fixed on aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/54aa371f173b
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
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
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?
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.
Checked in to aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/246acc814f72
Checked in to inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/63b8853b1d67 Thanks for the review!
I take it all that I'm documenting here is that addEventListnener() now returns as if nothing's wrong if the listener is null?
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.