cleanup command line handling

VERIFIED FIXED

Status

Fennec Graveyard
General
VERIFIED FIXED
8 years ago
6 years ago

People

(Reporter: fabrice, Assigned: fabrice)

Tracking

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 472356 [details] [diff] [review]
patch

Currently, command line handling is shared between the BrowserCLH.js component and the startup method in browser.js

This leads to code duplication, especially when adding web apps support.

This patch relocates most of the code in BrowserCLH.js.
(Assignee)

Comment 1

8 years ago
Created attachment 472386 [details] [diff] [review]
patch
Assignee: nobody → fabrice
Attachment #472356 - Attachment is obsolete: true
Attachment #472386 - Flags: review?(mark.finkle)
(Assignee)

Updated

8 years ago
Blocks: 584767
Attachment #472386 - Flags: review?(mark.finkle) → review+
Comment on attachment 472386 [details] [diff] [review]
patch

I found a problem where the about:home page is always opened when using | fennec someurl | and I don't think we want that. We should move opening the default page into BrowserCLH.js too.
Attachment #472386 - Flags: review+ → review-
(Assignee)

Comment 3

8 years ago
Created attachment 473025 [details] [diff] [review]
new patch

This version fixes the issue in comment 2
Attachment #472386 - Attachment is obsolete: true
Attachment #473025 - Flags: review?(mark.finkle)
Comment on attachment 473025 [details] [diff] [review]
new patch

Not quite what I had in mind. "firstArg" is not good enough to handle all the different launch cases.

I have a new patch based on your first patch that might show what I mean better.
Attachment #473025 - Flags: review?(mark.finkle) → review-
Created attachment 473047 [details] [diff] [review]
more patch

This patch is based on Fabrice's first patch. This patch does a little more:
* Moves all default page handling to BrowserCLH.js
* When creating the main browser window in the CLH, we now pass the default page as an argument
* The default page is the only thing handled in Browser now.
* Adds more comments to the CLH code to help make it clear what is happening
* Leaves a copy of getHomepage in Browser because other front-end can and does use it

This patch handles the following use cases where Fennec is not already running:
* | fennec | (firstrun or homepage is correctly shown)
* | fennec -url xxx | (xxx is loaded instead of firstrun or homepage. no other tabs are opened)
* | fennec -url xxx yyy zzz | (xxx is loaded instead of firstrun or homepage. yyy and zzz are opened in new tabs)

If Fennec is already running:
* | fennec | (fennec window is focused. no new tabs opened)
* | fennec -url xxx yyy zzz | (fennec window is focused. xxx, yyy and zzz are opened in new tabs)
Attachment #473025 - Attachment is obsolete: true
Attachment #473047 - Flags: review?(fabrice)
(Assignee)

Updated

8 years ago
Attachment #473047 - Flags: review?(fabrice) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/0a7c68052c8a
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Verified as fixed on Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110914 Firefox/9.0a1 Fennec/9.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.