Closed
Bug 994659
Opened 11 years ago
Closed 11 years ago
Exceptions in all command line code when reloading shell.html
Categories
(Firefox OS Graveyard :: Runtime, defect)
Firefox OS Graveyard
Runtime
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #8404685 -
Attachment is obsolete: true
Attachment #8404686 -
Flags: review?(21)
Assignee | ||
Updated•11 years ago
|
Blocks: firefox-mulet
Comment 3•11 years ago
|
||
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; } ?
Assignee | ||
Comment 4•11 years ago
|
||
(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.
Attachment #8404686 -
Flags: review?(21)
Assignee | ||
Comment 5•11 years ago
|
||
Use if (window.arguments) in runapp.js and desktop.js.
Attachment #8404686 -
Attachment is obsolete: true
Attachment #8404802 -
Flags: review?(21)
Attachment #8404802 -
Flags: review?(21) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Assignee | ||
Comment 7•11 years ago
|
||
I don't know why, but it hasn't run any tests, new try:
https://tbpl.mozilla.org/?tree=Try&rev=8ed6fd6bdccf
Assignee | ||
Comment 8•11 years ago
|
||
Ok, try doesn't want to run all tests, but at least Gu is green!
Keywords: checkin-needed
Comment 9•11 years ago
|
||
Keywords: checkin-needed
Comment 10•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•