Last Comment Bug 380618 - The function assigned to window.ononline gets called when the online event is fired (same for window.onoffline)
: The function assigned to window.ononline gets called when the online event is...
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla1.9alpha8
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
Mentors:
Depends on: 336682
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-14 01:31 PDT by Nickolay_Ponomarev
Modified: 2007-09-08 23:08 PDT (History)
10 users (show)
jonas: blocking1.9+
asqueella: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
unclean testcase; no FAIL lines indicates success (1.62 KB, text/html)
2007-05-14 01:31 PDT, Nickolay_Ponomarev
no flags Details
Fix, remove support for window.on{on|off}line (2.37 KB, patch)
2007-07-03 15:25 PDT, Johnny Stenback (:jst, jst@mozilla.com)
cajbir.bugzilla: review+
jonas: superreview+
Details | Diff | Splinter Review

Description Nickolay_Ponomarev 2007-05-14 01:31:58 PDT
Created attachment 264723 [details]
unclean testcase; no FAIL lines indicates success

See bug 336682 comment 24 and the attached testcase.

From comment 24:
> Er, really?  Does that break pages that try to have a |var ononline| or that
> assign something like a number to a global ononline variable?  See bug 336359
> comment 12.
> 
> If it does, is that really what we want?

The testcase here indicates that window.ononline/onoffline is treated like a handler when assigned a function, albeit with the quirkiness of bug 380538.

I don't see how this breaks pages that assign numbers/booleans to global vars named ononline/onoffline, but I could have missed something.
Comment 1 Boris Zbarsky [:bz] 2007-05-14 01:40:59 PDT
If it doesn't break such pages (i.e. if the properties are replaceable), then I don't think we have a problem, really...  Need to figure out whether it does break them or not, basically.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-14 09:46:07 PDT
If there are other ways for authors to do this, do we really want to pollute the global namespace every time we add new events?
Comment 3 Nickolay_Ponomarev 2007-05-14 09:59:38 PDT
(In reply to comment #1)
> If it doesn't break such pages (i.e. if the properties are replaceable)

Well, what I did was doing
var ononline = 1;
directly in a <script> and then checking the value later in a load handler (after the testcase triggers the events, not that it should matter). Anything else needs to be checked?

If you have a list of uses we shouldn't break with new window.on* handleres, they could be turned into a mochitest.
Comment 4 Jonas Sicking (:sicking) No longer reading bugmail consistently 2007-05-15 15:43:39 PDT
Jst will figure out what to do :)
Comment 5 Nickolay_Ponomarev 2007-06-21 12:12:28 PDT
content/events/test/test_bug336682.js has 'todo()'s because of this bug. However this bug is resolved, that test should be updated.
Comment 6 Damon Sicore (:damons) 2007-06-28 14:02:07 PDT
Setting to B1 per conversation with JST.
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2007-07-03 15:11:30 PDT
Per discussion with Chris Double and Dave Camp, we'll be removing the support for window.ononline and window.onoffline in favor of the standard and more flexible window.addEventListener() method of registering listeners for these new events. Patch coming up.
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2007-07-03 15:25:07 PDT
Created attachment 270792 [details] [diff] [review]
Fix, remove support for window.on{on|off}line
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2007-07-03 16:41:22 PDT
Fix checked in.
Comment 10 Boris Zbarsky [:bz] 2007-07-14 01:01:30 PDT
With this patch, did we stop supporting |body.ononline = function () {}| as well?  If so, is that what we want to do?

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