809392 - The remote and chrome debugger windows don't have a title

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Comment 1

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

Attachment: v1

Comment 2

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

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

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

::: browser/devtools/debugger/debugger-view.js
@@ +33,5 @@
>      if (window._isRemoteDebugger || window._isChromeDebugger) {
>        window.moveTo(Prefs.windowX, Prefs.windowY);
>        window.resizeTo(Prefs.windowWidth, Prefs.windowHeight);
>      }
> +    if (window._isRemoteDebugger) {

Tiny perf win: move the conditionals inside the previous one and use 'else' for the second branch.

@@ +37,5 @@
> +    if (window._isRemoteDebugger) {
> +      document.documentElement.setAttribute("title", L10N.getStr("remoteDebuggerWindowTitle"));
> +    }
> +    if (window._isChromeDebugger) {
> +      document.documentElement.setAttribute("title", L10N.getStr("chromeDebuggerWindowTitle"));

Doesn't document.title work in this case?

Comment 3

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

(In reply to Panos Astithas [:past] from comment #2)
> Comment on attachment 679101 [details] [diff] [review]
> v1
> 
> Review of attachment 679101 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/debugger/debugger-view.js
> @@ +33,5 @@
> >      if (window._isRemoteDebugger || window._isChromeDebugger) {
> >        window.moveTo(Prefs.windowX, Prefs.windowY);
> >        window.resizeTo(Prefs.windowWidth, Prefs.windowHeight);
> >      }
> > +    if (window._isRemoteDebugger) {
> 
> Tiny perf win: move the conditionals inside the previous one and use 'else'
> for the second branch.

Ok.

> 
> @@ +37,5 @@
> > +    if (window._isRemoteDebugger) {
> > +      document.documentElement.setAttribute("title", L10N.getStr("remoteDebuggerWindowTitle"));
> > +    }
> > +    if (window._isChromeDebugger) {
> > +      document.documentElement.setAttribute("title", L10N.getStr("chromeDebuggerWindowTitle"));
> 
> Doesn't document.title work in this case?

Gosh, I'm overcomplicating things...

Comment 4

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

Attachment: v2

Addressed comments.

Comment 5

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

https://hg.mozilla.org/integration/fx-team/rev/0a4944989068

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a4944989068