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?
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.
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.
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...
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.
Assignee: nobody → jonas
Created attachment 551529 [details] [diff] [review] Patch to fix
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!
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?
Checked in to aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/246acc814f72
status-firefox8: --- → fixed
Attachment #551558 - Flags: review?(Olli.Pettay) → review+
Checked in to inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/63b8853b1d67 Thanks for the review!
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.