750872 - Add yet more GCLI tests, this time for developer toolbar

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Comment 1

[Joe Walker [:jwalker]]

Attachment: Upload 1

I took this out of the mega-patch earlier to make it less mega.
Please could you take a particularly close look at oneTimeObserve().
Thanks.

Comment 2

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 620054 [details] [diff] [review]
Upload 1

+
+  // Christmas tree!

! :)

+function oneTimeObserve(name, callback) {
+  var func = function() {
+    Services.obs.removeObserver(func, name, false);

Don't need the third argument on removeObserver (it takes 2).

+    console.log('OTO end ' + name);

probably unneeded now?

+    callback();

I might use callback.call() just to be a pedant, but it's your call. (see what I did there?)

Comment 3

[Joe Walker [:jwalker]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #2)
> Comment on attachment 620054 [details] [diff] [review]

> +function oneTimeObserve(name, callback) {
> +  var func = function() {
> +    Services.obs.removeObserver(func, name, false);
> 
> Don't need the third argument on removeObserver (it takes 2).

Ok, thanks. I smell some confusion with addEventListener.

> +    console.log('OTO end ' + name);
> 
> probably unneeded now?

Yes.

> +    callback();
> 
> I might use callback.call() just to be a pedant, but it's your call. (see
> what I did there?)

Is there any technical difference between the 2? - I'm happy to change it, but adding chars seems like it needs a justification!

Thanks,

Joe.

Comment 4

[Joe Walker [:jwalker]]

Attachment: Upload 2

Fixes Rob's comments

Comment 5

[Joe Walker [:jwalker]]

Relevant tries:
https://tbpl.mozilla.org/?tree=Try&rev=77b47c8bbc35
https://tbpl.mozilla.org/?tree=Try&rev=1ce36097dc58

Comment 6

[Joe Walker [:jwalker]]

Try before land:
https://tbpl.mozilla.org/?tree=Try&rev=eba127ab7962

Fx-Team:
https://tbpl.mozilla.org/?tree=Fx-Team&rev=0f08ab4dfcac

Comment 7

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/0f08ab4dfcac