Last Comment Bug 883311 - Add Tab List implementation for the remote debugger
: Add Tab List implementation for the remote debugger
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 24.0
Assigned To: Philipp Kewisch [:Fallen]
:
Mentors:
Depends on:
Blocks: tb-debugger
  Show dependency treegraph
 
Reported: 2013-06-14 12:12 PDT by Philipp Kewisch [:Fallen]
Modified: 2013-06-25 05:18 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix - v1 (6.54 KB, patch)
2013-06-14 12:14 PDT, Philipp Kewisch [:Fallen]
mconley: review+
Details | Diff | Splinter Review

Description Philipp Kewisch [:Fallen] 2013-06-14 12:12:06 PDT
Aside from the main process, the remote debugger supports "tabs", which in Firefox are the real window tabs. I think the closest we can get to this is to provide access to the content windows inside <browser> elements.
Comment 1 Philipp Kewisch [:Fallen] 2013-06-14 12:14:05 PDT
Created attachment 762839 [details] [diff] [review]
Fix - v1
Comment 2 Mike Conley (:mconley) - (Needinfo me!) 2013-06-16 12:44:52 PDT
Comment on attachment 762839 [details] [diff] [review]
Fix - v1

Review of attachment 762839 [details] [diff] [review]:
-----------------------------------------------------------------

This is really straight-forward. Thanks Philipp.

Just some formatting nits, and maybe use a Set checkedWindows instead of an Array.

::: mail/components/debugger/content/dbg-mail-actors.js
@@ +61,5 @@
> +  _mustNotify: false,
> +  _listeningToMediator: false,
> +
> +  // These windows should be checked for browser elements
> +  checkedWindows: ["mail:3pane", "mail:messageWindow"],

If we're going to store _actorByBrowser as a Map, we might as well go the full nine yards and store checkedWindows as a Set.  Then use checkedWindows.has() instead of indexOf > -1.

@@ +64,5 @@
> +  // These windows should be checked for browser elements
> +  checkedWindows: ["mail:3pane", "mail:messageWindow"],
> +
> +  get onListChanged() {
> +      return this._onListChanged;

2 space indentation please

@@ +76,5 @@
> +    this._checkListening();
> +  },
> +
> +  _checkListening: function() {
> +   let shouldListenToMediator =

2 space indentation
Comment 3 Philipp Kewisch [:Fallen] 2013-06-16 13:39:51 PDT
Pushed to comm-central changeset a17d8582694a
Comment 4 Philipp Kewisch [:Fallen] 2013-06-16 13:41:13 PDT
All comments taken care of, I went with the Set.

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