The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
General
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Robert Kaiser, Assigned: Robert Kaiser)

Tracking

Trunk
seamonkey2.1a1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
+++ 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.
(Assignee)

Comment 1

7 years ago
Actually, I'm reverting the order between this and bug 547815 as bug 543444 changes the interfaces and therefore breaks us right now.
Blocks: 547815
No longer depends on: 547815
(Assignee)

Comment 2

7 years ago
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.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #430425 - Flags: review?(neil)
(Assignee)

Updated

7 years ago
Severity: normal → blocker
Target Milestone: --- → seamonkey2.1a1
Comment on attachment 430425 [details] [diff] [review]
patch to port us over to the interface changes

Looks good to me FWIW.
Attachment #430425 - Flags: review+

Comment 4

7 years ago
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
Attachment #430425 - Flags: review?(neil) → review+
(Assignee)

Comment 5

7 years ago
(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...
(Assignee)

Comment 6

7 years ago
back to normal, as the toolkit side got backout out (again).
Severity: blocker → normal
(Assignee)

Comment 7

7 years ago
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.
Attachment #430425 - Attachment is obsolete: true
Attachment #431854 - Flags: review?(neil)

Comment 8

7 years ago
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.
Attachment #431854 - Flags: review?(neil) → review+
(Assignee)

Comment 9

7 years ago
Thanks, pushed as http://hg.mozilla.org/comm-central/rev/1ae4523a0dc5
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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.
Attachment #658611 - Flags: review?(iann_bugzilla)

Updated

5 years ago
Attachment #658611 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 658611 [details] [diff] [review]
Missed one spot

Pushed comm-central changeset 1bdd4637a53c.
You need to log in before you can comment on or make changes to this bug.