If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsITimerCallback should be declared as [function]

RESOLVED FIXED

Status

()

Core
XPCOM
--
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: BenB, Assigned: BenB)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

10 years ago
nsITimerCallback is just a callback, as the same says, with only one function.
XPConnect has a nice feature to make usage of these from JavaScript much more comfortable: If you add "function" to the interface definition, the JS caller can just pass a normal JS function object instead of an object of this interface, and XPConnect will wrap it and call the JS function when the first method of the interface (in this case "notify") is called.

Actual result:
 Interface:
[scriptable, uuid(a796816d-7d47-4348-9ab8-c7aeb3216a7d)]
interface nsITimerCallback : nsISupports

 Usage:
function ttRunAsyncTimerCallback(func)
{
    assert(typeof func == "function", "please pass a JS function");
    this._func = func;
}
ttRunAsyncTimerCallback.prototype =
{
    _func : null,
    notify : function(timer)
    {
        this._func();
    },
    QueryInterface : function(iid)
    {
        if (iid.equals(CI.nsITimerCallback) ||
            iid.equals(CI.nsISupports))
            return this;
       throw Components.results.NS_NOINTERFACE;
    }
}

...
    var func = function() {...};
    var timer = Cc["@mozilla.org/timer;1"].createInstance(CI.nsITimer);
    timer.initWithCallback(new ttRunAsyncTimerCallback(func), sec * 1000,
        CI.nsITimer.TYPE_ONE_SHOT);

Expected result:
 Interface:
[function, scriptable, uuid(a796816d-7d47-4348-9ab8-c7aeb3216a7d)]
interface nsITimerCallback : nsISupports

 Usage:
    var func = function() {...};
    var timer = Cc["@mozilla.org/timer;1"].createInstance(CI.nsITimer);
    timer.initWithCallback(func, sec * 1000, CI.nsITimer.TYPE_ONE_SHOT);
(Assignee)

Comment 1

10 years ago
Created attachment 317052 [details] [diff] [review]
Fix
Assignee: nobody → ben.bucksch
Status: NEW → ASSIGNED
Attachment #317052 - Flags: review?
(Assignee)

Comment 2

10 years ago
Comment on attachment 317052 [details] [diff] [review]
Fix

Tested, works
Attachment #317052 - Attachment description: Fix (untested) → Fix
Attachment #317052 - Flags: review? → review?(shaver)
Comment on attachment 317052 [details] [diff] [review]
Fix

Put the tests in this bug, pls (make-check style preferred).
(Assignee)

Comment 4

10 years ago
Created attachment 317091 [details]
Test, for JS function
(Assignee)

Comment 5

10 years ago
Created attachment 317092 [details]
Test, to still be able to pass an IDL object
Attachment #317091 - Attachment mime type: application/javascript → text/plain
Attachment #317092 - Attachment mime type: application/javascript → text/plain
(Assignee)

Updated

10 years ago
Blocks: 393966
(Assignee)

Comment 6

10 years ago
Revving IID is not necessary:
[23:05:19] <bz> weirdal: see logs if they exist?  bsmedberg, timeless, and I all agree that this should be fine
(Assignee)

Comment 7

10 years ago
Created attachment 317208 [details] [diff] [review]
Fix and tests

All in one patch
Attachment #317052 - Attachment is obsolete: true
Attachment #317091 - Attachment is obsolete: true
Attachment #317092 - Attachment is obsolete: true
Attachment #317208 - Flags: review?
Attachment #317052 - Flags: review?(shaver)
(Assignee)

Updated

10 years ago
Attachment #317208 - Flags: review? → review?(shaver)
Comment on attachment 317208 [details] [diff] [review]
Fix and tests

r+sr+a=shaver for this thoroughly-analyzed (additive) API fix, presuming that it passes the test suites.  (I'd have put both those test cases in the same file, but whatever!)
Attachment #317208 - Flags: superreview+
Attachment #317208 - Flags: review?(shaver)
Attachment #317208 - Flags: review+
Attachment #317208 - Flags: approval1.9+
(Assignee)

Comment 9

10 years ago
Thanks for the review and approval!
Checked in on trunk.
FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.