Last Comment Bug 740467 - Make test_XHR_timeout.js use addEventListener(..., obj, ...) instead of on* = obj
: Make test_XHR_timeout.js use addEventListener(..., obj, ...) instead of on* =...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Peter Van der Beken [:peterv]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-29 10:30 PDT by Peter Van der Beken [:peterv]
Modified: 2012-03-30 13:05 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (1.82 KB, patch)
2012-03-29 10:30 PDT, Peter Van der Beken [:peterv]
khuey: review+
Details | Diff | Splinter Review
v2 (1.84 KB, patch)
2012-03-29 10:58 PDT, Peter Van der Beken [:peterv]
khuey: review+
Details | Diff | Splinter Review

Description Peter Van der Beken [:peterv] 2012-03-29 10:30:41 PDT
Created attachment 610585 [details] [diff] [review]
v1
Comment 1 Alex Vincent [:WeirdAl] 2012-03-29 10:37:32 PDT
I object to this.  I had a reason for using on*.  The spec said the ontimeout attribute had to exist and map to a timeout event listener.  This patch removes the test for ontimeout working.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-29 10:41:14 PDT
Ok, then we need to test that differently.  Per spec onfoo properties don't accept objects with handleEvent methods, only functions.
Comment 3 Olli Pettay [:smaug] 2012-03-29 10:43:51 PDT
Yeah, kyle is right.  I should have noticed the problem in the xhr.timeout tests.
Comment 4 Alex Vincent [:WeirdAl] 2012-03-29 10:46:44 PDT
I agree - the code should've caught that... but I think that should be a separate bug.

I'd feel better if the test file itself were cloned, and the clone used the addEventListener approach.
Comment 5 Peter Van der Beken [:peterv] 2012-03-29 10:58:52 PDT
Created attachment 610603 [details] [diff] [review]
v2
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-29 11:02:55 PDT
To be clear, when we land bug 740069 this test will stop working if we don't do anything to it.

Peterv is going to post a new patch that addresses your comments.
Comment 7 Peter Van der Beken [:peterv] 2012-03-30 09:43:51 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/5561efcb19f2
Comment 8 Ed Morley [:emorley] 2012-03-30 13:05:17 PDT
https://hg.mozilla.org/mozilla-central/rev/5561efcb19f2

Note You need to log in before you can comment on or make changes to this bug.