Closed Bug 647571 Opened 13 years ago Closed 13 years ago

Add Places Bookmarks interfaces to SMILE

Categories

(SeaMonkey :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b3

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(2 files)

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.
> -[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
Attachment #523889 - Flags: review?(neil)
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?]
Attachment #523889 - Flags: review?(neil) → review+
>>+      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
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
Attachment #523954 - Attachment is patch: true
Attachment #523954 - Attachment mime type: application/octet-stream → text/plain
Blocks: 647654
> [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.
Flags: in-testsuite+
Depends on: 758102
You need to log in before you can comment on or make changes to this bug.