550234 - Port bug 543444 (Replace single-view API with multiple observers) to SeaMonkey history

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Robert Kaiser]

+++ 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]

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]

Attachment: 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

[Asaf Romano (gone)]

Comment on attachment 430425 [details] [diff] [review]
patch to port us over to the interface changes

Looks good to me FWIW.

Comment 4

[[e-mail]]

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]

(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]

back to normal, as the toolkit side got backout out (again).

Comment 7

[Robert Kaiser]

Attachment: 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

[[e-mail]]

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]

Thanks, pushed as http://hg.mozilla.org/comm-central/rev/1ae4523a0dc5

Comment 10

[[e-mail]]

Attachment: 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

[[e-mail]]

Comment on attachment 658611 [details] [diff] [review]
Missed one spot

Pushed comm-central changeset 1bdd4637a53c.