Bug 1317580 - Add additional deeplink support for FxA #2249
Conversation
vbudhram
changed the title
|
Thanks for this PR |
| @@ -1968,11 +1968,13 @@ extension BrowserViewController: HomePanelViewControllerDelegate { | |||
| } | |||
|
|
|||
| func homePanelViewControllerDidRequestToCreateAccount(homePanelViewController: HomePanelViewController) { | |||
| presentSignInViewController() // TODO UX Right now the flow for sign in and create account is the same | |||
| let fxaParams: [String: String] = ["view" : "signup"] | |||
farhanpatel
Nov 23, 2016
Contributor
you can skip the [String: String] here .
you can skip the [String: String] here .
| } | ||
|
|
||
| func homePanelViewControllerDidRequestToSignIn(homePanelViewController: HomePanelViewController) { | ||
| presentSignInViewController() // TODO UX Right now the flow for sign in and create account is the same | ||
| let fxaParams: [String: String] = ["view" : "signin"] |
farhanpatel
Nov 23, 2016
Contributor
Same as above 😄
Same as above
| "view" : "", | ||
| "email" : "", | ||
| "code" : "" | ||
| ] |
farhanpatel
Nov 23, 2016
Contributor
For something like this it might be simpler to use a struct. You'll be able to use dot notation and better type checking.
For something like this it might be simpler to use a struct. You'll be able to use dot notation and better type checking.
| @@ -291,8 +304,13 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
|
|
|||
| func launchFromURL(params: LaunchParams) { | |||
| let isPrivate = params.isPrivate ?? false | |||
| let fxaParams = params.fxaParams ?? nil | |||
farhanpatel
Nov 23, 2016
Contributor
I think it'll be simpler if you just make a new method instead of using the existing LaunchFromURL. That way you dont need to check for a valid fxaParams here.
I think it'll be simpler if you just make a new method instead of using the existing LaunchFromURL. That way you dont need to check for a valid fxaParams here.
| for item in (components.queryItems ?? []) as [NSURLQueryItem] { | ||
| switch item.name { | ||
| case "url": | ||
| url = item.value | ||
| case "private": | ||
| isPrivate = NSString(string: item.value ?? "false").boolValue | ||
| case "fxa": |
farhanpatel
Nov 23, 2016
Contributor
I know this loop/switch is already here. But using url.getQuery() will return a dictionary and prevent you from having to loop through results like this.
I know this loop/switch is already here. But using url.getQuery() will return a dictionary and prevent you from having to loop through results like this.
| @@ -614,4 +632,5 @@ extension AppDelegate: MFMailComposeViewControllerDelegate { | |||
| struct LaunchParams { | |||
| let url: NSURL? | |||
| let isPrivate: Bool? | |||
| let fxaParams: NSDictionary? | |||
farhanpatel
Nov 23, 2016
Contributor
if you create your own LaunchFxAWithURL you can create your own FxAStruct 😄
if you create your own LaunchFxAWithURL you can create your own FxAStruct
|
@farhanpatel A little new to Swift, but I think I got your changes in. Does this look correct? r? |
| // Extract optional FxA deep-linking options | ||
| let fxaQuery = url.getQuery() | ||
| let fxaParams: FxALaunchParams | ||
| fxaParams = FxALaunchParams(view: fxaQuery["fxa"], email: fxaQuery["email"], access_code: fxaQuery["access_code"]) |
st3fan
Dec 13, 2016
Contributor
Will this crash if fxaQuery does not contain any of those fields?
Will this crash if fxaQuery does not contain any of those fields?
st3fan
Dec 13, 2016
Contributor
(Not sure if looking up an non existent entry will abort/crash or not)
(Not sure if looking up an non existent entry will abort/crash or not)
vbudhram
Dec 13, 2016
Author
Contributor
Hey @st3fan, It doesn't appear so. I opened the following deep links in safari and got the following results. These seem to be correct.
firefox://?fxa=signin&email=test@email.com
Launches FxiOS, opens the FxA Content View Controller and prefills email.
firefox://?fxa=signin
Launches FxiOS, opens the FxA Content View Controller
firefox://
Launches FxiOS
Hey @st3fan, It doesn't appear so. I opened the following deep links in safari and got the following results. These seem to be correct.
firefox://?fxa=signin&email=test@email.com
Launches FxiOS, opens the FxA Content View Controller and prefills email.
firefox://?fxa=signin
Launches FxiOS, opens the FxA Content View Controller
firefox://
Launches FxiOS
farhanpatel
Dec 13, 2016
Contributor
Yea it should be fine 👍 You've declared the launch params as optional so it shouldn't crash. They are allowed to be nil
Yea it should be fine
|
Almost there! I hope the changes make sense. |
| return | ||
| } | ||
| let script = "$('.\(className!)').val('\(value!)');"; | ||
| webView.evaluateJavaScript(script, completionHandler: { (res, error) -> Void in |
farhanpatel
Dec 13, 2016
Contributor
You can pass nll to the completionHandler to avoid declaring it.
You can pass nll to the completionHandler to avoid declaring it.
|
|
||
| // Attempt to fill out a field in content view | ||
| private func fillField(className: String?, value: String?){ | ||
| if(className == nil || value == nil){ |
farhanpatel
Dec 13, 2016
Contributor
you can use a guard statement here instead guard let name = className, let val = value else { return }
you can use a guard statement here instead guard let name = className, let val = value else { return }
farhanpatel
Dec 13, 2016
Contributor
We usually prefer guard but tbh in this case I think either is fine. Make sure to add a space between the ) and {
We usually prefer guard but tbh in this case I think either is fine. Make sure to add a space between the ) and {
| @@ -288,6 +299,13 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
|
|
|||
| return true | |||
| } | |||
|
|
|||
| func launchFxAFromURL(params: FxALaunchParams) { | |||
| let view = params.view ?? nil | |||
farhanpatel
Dec 13, 2016
Contributor
A guard statement here would work better guard let view = params.view else { return }
A guard statement here would work better guard let view = params.view else { return }
|
@farhanpatel Thanks for feedback, made changes. |
| let fxaParams: FxALaunchParams | ||
| fxaParams = FxALaunchParams(view: fxaQuery["fxa"], email: fxaQuery["email"], access_code: fxaQuery["access_code"]) | ||
|
|
||
| if(fxaParams.view != nil){ |
farhanpatel
Dec 13, 2016
Contributor
can you please add spaces between the parans
can you please add spaces between the parans
This PR adds some custom deep linking support for Firefox Accounts. Specifically, this allows accounts to launch the
FxAContentViewControllerdirectly and prefill the form with an email and access_code.The updated deeplink scheme looks like this.
firefox://?fxa=signin&email=test@email.com&access_code=12345This update does not change previous deeplinking functionality. If FxA options are not specified, it will work as previously expected.