994659 - Exceptions in all command line code when reloading shell.html

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Runtime, defect)

Description

[Alexandre Poirot [:ochameau]]

Reloading shell.html isn't something we are used to do.
But it is going to change when using devtools and Firefox Mulet.

We see exceptions in all components that implement b2g specific command line arguments (runapp.js, screen.js and desktop.js).
Just because they assume window.arguments contains a nsICommandLine element,
which isn't true when you reload the page -or- open shell.html manually within the mulet.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Set shell.html as homepage

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Fix exceptions when reloading shell.html

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8404686 [details] [diff] [review]
Fix exceptions when reloading shell.html

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

::: b2g/chrome/content/desktop.js
@@ +58,4 @@
>    let dbgport;
>    try {
> +    // Get the command line arguments that were passed to the b2g client
> +    let args = window.arguments[0].QueryInterface(Ci.nsICommandLine);

Can't you just: if (!window.arguments.length) { return; } ?

::: b2g/chrome/content/runapp.js
@@ +12,5 @@
> +    args = window.arguments[0].QueryInterface(Ci.nsICommandLine);
> +  } catch(e) {}
> +  if (!args) {
> +    return;
> +  }

Can't you just: if (!window.arguments.length) { return; } ?

::: b2g/chrome/content/screen.js
@@ +63,5 @@
> +  let args;
> +  try {
> +    // On Firefox Mulet, we don't always have a command line argument
> +    args = window.arguments[0].QueryInterface(Ci.nsICommandLine);
> +  } catch(e) {}

Can't you just: if (!window.arguments.length) { return; } ?

Comment 4

[Alexandre Poirot [:ochameau]]

(In reply to Vivien Nicolas (:vingtetun) (:21) (NOT reading bugmails, needinfo? please) from comment #3)
> Comment on attachment 8404686 [details] [diff] [review]
> Fix exceptions when reloading shell.html
> 
> Review of attachment 8404686 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/chrome/content/desktop.js
> @@ +58,4 @@
> >    let dbgport;
> >    try {
> > +    // Get the command line arguments that were passed to the b2g client
> > +    let args = window.arguments[0].QueryInterface(Ci.nsICommandLine);
> 
> Can't you just: if (!window.arguments.length) { return; } ?

Yes

> 
> ::: b2g/chrome/content/runapp.js
> @@ +12,5 @@
> > +    args = window.arguments[0].QueryInterface(Ci.nsICommandLine);
> > +  } catch(e) {}
> > +  if (!args) {
> > +    return;
> > +  }

Yes

> 
> Can't you just: if (!window.arguments.length) { return; } ?
> 
> ::: b2g/chrome/content/screen.js
> @@ +63,5 @@
> > +  let args;
> > +  try {
> > +    // On Firefox Mulet, we don't always have a command line argument
> > +    args = window.arguments[0].QueryInterface(Ci.nsICommandLine);
> > +  } catch(e) {}
> 
> Can't you just: if (!window.arguments.length) { return; } ?

No! the code coming after needs to be executed to have a correct window size.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Fix exceptions when reloading shell.html

Use if (window.arguments) in runapp.js and desktop.js.

Comment 6

[Alexandre Poirot [:ochameau]]

https://tbpl.mozilla.org/?tree=Try&rev=26c4c372d5bb

Comment 7

[Alexandre Poirot [:ochameau]]

I don't know why, but it hasn't run any tests, new try:
https://tbpl.mozilla.org/?tree=Try&rev=8ed6fd6bdccf

Comment 8

[Alexandre Poirot [:ochameau]]

Ok, try doesn't want to run all tests, but at least Gu is green!

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/c4635052e316

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c4635052e316