768677 - Remote debugger asks for host:port, displays http://host:port/ as default value

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Philipp von Weitershausen [:philikon]]

http://cdn.memegenerator.net/instances/400x/22557117.jpg

Comment 1

[Victor Porof [:vporof][:vp]]

Wow... how did we not notice this until now?!

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

LOL

Comment 3

[Victor Porof [:vporof][:vp]]

Attachment: v1

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 638668 [details] [diff] [review]
v1

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

Although I agree with the spirit of these changes, overall this patch is a regression, since the prompt cannot be dismissed now by clicking on Cancel.

Comment 5

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

would like to get this before the merge on monday.

Comment 6

[Victor Porof [:vporof][:vp]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #5)
> would like to get this before the merge on monday.

Shall happen.

Comment 7

[Victor Porof [:vporof][:vp]]

Attachment: v1

There.

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 642340 [details] [diff] [review]
v1

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

Cancel now works, but there is still a brief flashing of the debugger window. Can't we get rid of that, too?

::: browser/devtools/debugger/debugger-view.js
@@ +120,5 @@
>  
> +      if (!result) {
> +        return false;
> +      }
> +      if ((parts = input.value.split(":")).length === 2) {

This is unnecessarily complicated, since you don't avoid the extra line of the |parts| declaration above :-)

Comment 9

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #8)
> Comment on attachment 642340 [details] [diff] [review]
> v1
> 
> Review of attachment 642340 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Cancel now works, but there is still a brief flashing of the debugger
> window. Can't we get rid of that, too?
> 
> ::: browser/devtools/debugger/debugger-view.js
> @@ +120,5 @@
> >  
> > +      if (!result) {
> > +        return false;
> > +      }
> > +      if ((parts = input.value.split(":")).length === 2) {
> 
> This is unnecessarily complicated, since you don't avoid the extra line of
> the |parts| declaration above :-)

The flashing was there before.
With the current implementation, it cannot be removed, since the prompt is blocking and debugger content window is not shown yet. Bug 751677 will take care of this.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Victor Porof from comment #9)
> With the current implementation, it cannot be removed, since the prompt is
> blocking and debugger content window is not shown yet. Bug 751677 will take
> care of this.

OK then.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/842865c36ff1

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/842865c36ff1