Make it possible to start a remote debugger only in a new chrome window

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vporof, Assigned: vporof)

Tracking

12 Branch
Firefox 15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
New process would make sense only for chrome debugging. In all other cases, a new window is nicer for tons of reasons.

Comment 1

5 years ago
Should this bug also include a debuggee/tab selection UI in this bug?
(Assignee)

Comment 2

5 years ago
(In reply to Dave Camp (:dcamp) from comment #1)
> Should this bug also include a debuggee/tab selection UI in this bug?

Why should it? We could just say "One debugger per tab. You pop it in a new window, it's the same debugger for the same debuggee. Want to debug another tab? Open a new debugger".

Comment 3

5 years ago
I misremembered this bug as being for general remote fennec/b2g debugging - I need to specify how to connect to the phone I intend to debug, and which of its debuggees I'm interested in.

I guess we could just always debug the currently-visible tab, but we do still need some way to specify the debuggee to connect to.
(In reply to Dave Camp (:dcamp) from comment #3)
> I misremembered this bug as being for general remote fennec/b2g debugging -
> I need to specify how to connect to the phone I intend to debug, and which
> of its debuggees I'm interested in.
> 
> I guess we could just always debug the currently-visible tab, but we do
> still need some way to specify the debuggee to connect to.

Filed bug 748927 for this.
(Assignee)

Comment 5

5 years ago
Note: we shouldn't be able to open a remote debugger in a new firefox instance. A simple new window should suffice.
(Assignee)

Updated

5 years ago
Summary: Make it possible to start a debugger in a new chrome window → Make it possible to start a remote debugger only in a new chrome window
(Assignee)

Updated

5 years ago
Assignee: nobody → vporof
Status: NEW → ASSIGNED
(Assignee)

Comment 6

5 years ago
Created attachment 619008 [details] [diff] [review]
v1

WIP
(Assignee)

Updated

5 years ago
Depends on: 749222
(Assignee)

Comment 7

5 years ago
Created attachment 619050 [details] [diff] [review]
v2

Works!
Attachment #619008 - Attachment is obsolete: true
Attachment #619050 - Flags: review?(past)
(Assignee)

Comment 8

5 years ago
Comment on attachment 619050 [details] [diff] [review]
v2

I herd Rob has free time on his hands.
Attachment #619050 - Flags: review?(past) → review?(rcampbell)
(Assignee)

Updated

5 years ago
Blocks: 748927
(In reply to Victor Porof from comment #8)
> Comment on attachment 619050 [details] [diff] [review]
> v2
> 
> I herd Rob has free time on his hands.

hey, thanks, buddy. I'll get right on this.
Created attachment 619125 [details] [diff] [review]
v2+x

augh
(Assignee)

Comment 11

5 years ago
Created attachment 619281 [details] [diff] [review]
v2+xx

Small changes, typos.
Attachment #619050 - Attachment is obsolete: true
Attachment #619125 - Attachment is obsolete: true
Attachment #619050 - Flags: review?(rcampbell)
Attachment #619281 - Flags: review?(rcampbell)
Comment on attachment 619281 [details] [diff] [review]
v2+xx

+   * Creates and initializes the widgets containing the remote debugger UI.
+   */
+  _create: function DP__create() {

I think in this method you should explicitly remove the toolbar's close button. It doesn't really make sense to have it there when in an external window. Also, it doesn't work.

Patch looks good, tests pass on my machine.

R+ with the above change.
Attachment #619281 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 13

5 years ago
(In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> Comment on attachment 619281 [details] [diff] [review]
> v2+xx
> 
> +   * Creates and initializes the widgets containing the remote debugger UI.
> +   */
> +  _create: function DP__create() {
> 
> I think in this method you should explicitly remove the toolbar's close
> button. It doesn't really make sense to have it there when in an external
> window. Also, it doesn't work.
> 
> Patch looks good, tests pass on my machine.
> 
> R+ with the above change.

Well not really in that method, it needs to be in the content js, not the jsm, because that's just a simple iframe wrapper.

Also, I guess this applies in the chrome debugging case as well, right?
(Assignee)

Comment 14

5 years ago
Created attachment 620771 [details] [diff] [review]
v3

Made the change.
(Assignee)

Comment 15

5 years ago
Created attachment 621626 [details] [diff] [review]
v3.1

Rebased.
Attachment #620771 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/5e3d7eccb832
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/5e3d7eccb832
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 15
You need to log in before you can comment on or make changes to this bug.