Closed Bug 994659 Opened 6 years ago Closed 6 years ago

Exceptions in all command line code when reloading shell.html

Categories

(Firefox OS Graveyard :: Runtime, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Set shell.html as homepage (obsolete) — Splinter Review
Attachment #8404685 - Attachment is obsolete: true
Attachment #8404686 - Flags: review?(21)
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; } ?
(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.
Use if (window.arguments) in runapp.js and desktop.js.
Attachment #8404686 - Attachment is obsolete: true
Attachment #8404802 - Flags: review?(21)
I don't know why, but it hasn't run any tests, new try:
https://tbpl.mozilla.org/?tree=Try&rev=8ed6fd6bdccf
Ok, try doesn't want to run all tests, but at least Gu is green!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c4635052e316
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.