Last Comment Bug 768677 - Remote debugger asks for host:port, displays http://host:port/ as default value
: Remote debugger asks for host:port, displays http://host:port/ as default value
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: Firefox 16
Assigned To: Victor Porof [:vporof][:vp]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-26 15:33 PDT by Philipp von Weitershausen [:philikon]
Modified: 2012-07-16 05:26 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.29 KB, patch)
2012-07-03 05:44 PDT, Victor Porof [:vporof][:vp]
past: review-
Details | Diff | Review
v1 (2.34 KB, patch)
2012-07-14 23:47 PDT, Victor Porof [:vporof][:vp]
past: review+
Details | Diff | Review

Description Philipp von Weitershausen [:philikon] 2012-06-26 15:33:30 PDT
http://cdn.memegenerator.net/instances/400x/22557117.jpg
Comment 1 Victor Porof [:vporof][:vp] 2012-06-28 09:34:42 PDT
Wow... how did we not notice this until now?!
Comment 2 Panos Astithas [:past] 2012-06-28 09:58:11 PDT
LOL
Comment 3 Victor Porof [:vporof][:vp] 2012-07-03 05:44:20 PDT
Created attachment 638668 [details] [diff] [review]
v1
Comment 4 Panos Astithas [:past] 2012-07-04 07:36:07 PDT
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) 2012-07-12 09:33:01 PDT
would like to get this before the merge on monday.
Comment 6 Victor Porof [:vporof][:vp] 2012-07-13 07:43:03 PDT
(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] 2012-07-14 23:47:40 PDT
Created attachment 642340 [details] [diff] [review]
v1

There.
Comment 8 Panos Astithas [:past] 2012-07-15 03:14:55 PDT
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] 2012-07-15 03:17:09 PDT
(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 [:past] 2012-07-15 03:24:57 PDT
(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 [:past] 2012-07-15 06:44:14 PDT
https://hg.mozilla.org/integration/fx-team/rev/842865c36ff1
Comment 12 Ed Morley [:emorley] 2012-07-16 05:26:39 PDT
https://hg.mozilla.org/mozilla-central/rev/842865c36ff1

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