Closed Bug 750872 Opened 8 years ago Closed 8 years ago

Add yet more GCLI tests, this time for developer toolbar

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 15

People

(Reporter: jwalker, Assigned: jwalker)

References

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch Upload 1 (obsolete) — Splinter Review
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.
Attachment #620054 - Flags: review?(rcampbell)
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?)
Attachment #620054 - Flags: review?(rcampbell) → review+
(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.
Attached patch Upload 2Splinter Review
Fixes Rob's comments
Attachment #620054 - Attachment is obsolete: true
Attachment #623231 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/0f08ab4dfcac
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.