Last Comment Bug 550234 - Port bug 543444 (Replace single-view API with multiple observers) to SeaMonkey history
: Port bug 543444 (Replace single-view API with multiple observers) to SeaMonke...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Robert Kaiser (not working on stability any more)
:
Mentors:
Depends on: 543444
Blocks: 547815
  Show dependency treegraph
 
Reported: 2010-03-04 10:34 PST by Robert Kaiser (not working on stability any more)
Modified: 2012-09-07 02:41 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch to port us over to the interface changes (10.04 KB, patch)
2010-03-04 14:50 PST, Robert Kaiser (not working on stability any more)
neil: review+
asaf: review+
Details | Diff | Review
v2: updated to recent bug 543444 changes (12.35 KB, patch)
2010-03-11 06:28 PST, Robert Kaiser (not working on stability any more)
neil: review+
Details | Diff | Review
Missed one spot (1.12 KB, patch)
2012-09-05 13:49 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2010-03-04 10:34:16 PST
+++ This bug was initially created as a clone of Bug #543444 +++

The browser/ changes in bug 543444 - Replace single-view API with multiple observers - should be ported to SeaMonkey history files.

I'll base the patch for it on the bug 547815 work.
Comment 1 Robert Kaiser (not working on stability any more) 2010-03-04 14:44:13 PST
Actually, I'm reverting the order between this and bug 547815 as bug 543444 changes the interfaces and therefore breaks us right now.
Comment 2 Robert Kaiser (not working on stability any more) 2010-03-04 14:50:25 PST
Created attachment 430425 [details] [diff] [review]
patch to port us over to the interface changes

This patch should port over everything and make us compatible with the changed interfaces.
Comment 3 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2010-03-04 14:58:39 PST
Comment on attachment 430425 [details] [diff] [review]
patch to port us over to the interface changes

Looks good to me FWIW.
Comment 4 neil@parkwaycc.co.uk 2010-03-04 15:27:13 PST
Comment on attachment 430425 [details] [diff] [review]
patch to port us over to the interface changes

>+    var suppressNotificationsOld = result.suppressNotifications;
Nit: I would have named it didSuppressNotifications

>-      var nodes = this._view.getDragableSelection();
>+      let nodes = this._view.getDragableSelection();
> 
>-      for (var i = 0; i < nodes.length; ++i) {
>+      for (let i = 0; i < nodes.length; ++i) {
Well I can't allow this given that you used var above :-P
Comment 5 Robert Kaiser (not working on stability any more) 2010-03-04 16:24:07 PST
(In reply to comment #4)
> (From update of attachment 430425 [details] [diff] [review])
> >+    var suppressNotificationsOld = result.suppressNotifications;
> Nit: I would have named it didSuppressNotifications

Hrm, do I have to do this? Renaming this makes porting of further changes much harder :(

> >-      var nodes = this._view.getDragableSelection();
> >+      let nodes = this._view.getDragableSelection();
> > 
> >-      for (var i = 0; i < nodes.length; ++i) {
> >+      for (let i = 0; i < nodes.length; ++i) {
> Well I can't allow this given that you used var above :-P

I don't understand... I'm trying hard to use var at the direct function level and let only in blocks underneath it - and in a for loop, let makes even more sense, IMHO...
Comment 6 Robert Kaiser (not working on stability any more) 2010-03-05 04:14:09 PST
back to normal, as the toolkit side got backout out (again).
Comment 7 Robert Kaiser (not working on stability any more) 2010-03-11 06:28:28 PST
Created attachment 431854 [details] [diff] [review]
v2: updated to recent bug 543444 changes

OK, I've updated the patch for recent changes to the bug 543444, including the variable renaming you requested.
Comment 8 neil@parkwaycc.co.uk 2010-03-11 08:43:35 PST
Comment on attachment 431854 [details] [diff] [review]
v2: updated to recent bug 543444 changes

Oh, I see now, the Firefox version has the same  var/let changes.
Comment 9 Robert Kaiser (not working on stability any more) 2010-03-12 08:07:14 PST
Thanks, pushed as http://hg.mozilla.org/comm-central/rev/1ae4523a0dc5
Comment 10 neil@parkwaycc.co.uk 2012-09-05 13:49:30 PDT
Created attachment 658611 [details] [diff] [review]
Missed one spot

I happened to spot the strict JS warning about the removed interface.

The failure doesn't break anything, but this call is quicker if it works.
Comment 11 neil@parkwaycc.co.uk 2012-09-07 02:41:52 PDT
Comment on attachment 658611 [details] [diff] [review]
Missed one spot

Pushed comm-central changeset 1bdd4637a53c.

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