647571 - Add Places Bookmarks interfaces to SMILE

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

Now that we are using the Places Bookmarks implementation we should update SMILE to support those and bring us up to feature parity with FUEL.

Comment 1

[Philip Chee]

Attachment: Patch v1.0 SMILEBookmarks.

> -[scriptable, uuid(c9ba8f65-c936-4ac6-a859-8936832b0c12)]
> +[scriptable, uuid(bedc9ea5-5673-44c7-a0af-49f1d37a2ca1)]
>  interface smileIApplication : extIApplication
q.v. [Bug 628586] steelIApplication needs new iid due to changes in parent interface extIApplication.

> +++ b/suite/smile/src/smileApplication.js
> 
> -  var self = this;
> -  gShutdown.push(function() { self._shutdown(); });
> +  gShutdown.push(this._shutdown.bind(this));
I hope I've done the bind correctly.

> -    var self = this;
> -    this._tabbrowser.addEventListener(aType,
> -      this._cleanup[aType] = function(e){ self._event(e); },
> +    this._tabbrowser.tabContainer.addEventListener(aType,
Firefox uses tabContainer due to tabs on top.

> +      this._cleanup[aType] = function(e){ this._event(e); }.bind(this),
>        true);
I hope I've done the bind correctly.

>    _shutdown : function win_shutdown() {
>      for (var type in this._cleanup)
> -      this._tabbrowser.removeEventListener(type, this._cleanup[type], true);
> +      this._tabbrowser.tabContainer.removeEventListener(type, this._cleanup[type], true);
Future proof a bit for when we implement tabs on top/tabs in a toolbar.

>    // for nsIObserver
>    observe: function app_observe(aSubject, aTopic, aData) {
>      // Call the extApplication version of this function first
>      this.__proto__.__proto__.observe.call(this, aSubject, aTopic, aData);
>      if (aTopic == "xpcom-shutdown") {
> +      this._obs.removeObserver(this, "xpcom-shutdown");
q.v. [Bug 506471] FUEL should avoid putting itself in the app-startup category (r=mfinkle).

Passes all the tests in browser_Bookmarks.js

Comment 2

[[e-mail]]

Comment on attachment 523889 [details] [diff] [review]
Patch v1.0 SMILEBookmarks.

>+      this._cleanup[aType] = function(e){ this._event(e); }.bind(this),
this._event.bind(this)

r=me with that fixed.

[In theory it's possible to avoid binding this._event N times by doing it once in the constructor. Followup bug perhaps?]

Comment 3

[Philip Chee]

Attachment: Patch v1.0a as checked in [Comment 3]

>>+      this._cleanup[aType] = function(e){ this._event(e); }.bind(this),
> this._event.bind(this)
> 
> r=me with that fixed.
>
Fixed.

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/99b127296682

Comment 4

[Philip Chee]

> [In theory it's possible to avoid binding this._event N times by doing it once
> in the constructor. Followup bug perhaps?]

Filed Bug 647654.