Refactor url handling (deep-link, open-url, fxa-signin) code

RESOLVED DUPLICATE of bug 1355636

Status

()

Firefox for iOS
General
P1
normal
RESOLVED DUPLICATE of bug 1355636
11 months ago
3 months ago

People

(Reporter: st3fan, Assigned: bkmunar)

Tracking

unspecified
Other
iOS
Dependency tree / graph

Firefox Tracking Flags

(fxios8.0+)

Details

(Whiteboard: [MobileCore][mma][b3])

Attachments

(1 attachment)

PR
55 bytes, text/x-github-pull-request
Details | Review | Splinter Review
(Reporter)

Description

11 months ago
I'm not happy with how we now have three different things under the firefox:// scheme:

- firefox://fxa-signin?signin=<token>&someQuery=<data>...
- firefox://open-url?deep-link=https%253A...
- firefox://anything-here-does-not-matter?deeplink=https%253A...

This reflects in the code, which is handling the last two cases combined and seems all over the place. And the URL style for the last two is not enforced .. you can actually replace open-url with anything. This is not strict enough.

I would like to ask for three things:

- Introduce a firefox://deep-link?url= style url
- Enforce a firefox://open-url?url= style for the public API
- Refactor this code to have three clear separate blocks or functions to handle firefox://fxa-signin, firefox://open-url and firefox://deep-link.
(Reporter)

Updated

11 months ago
Whiteboard: [MobileCore][mma][b3]

Updated

11 months ago
Blocks: 1351446

Updated

11 months ago
Depends on: 1369554

Comment 1

11 months ago
(In reply to Stefan Arentz [:st3fan] from comment #0)
> I'm not happy with how we now have three different things under the
> firefox:// scheme:
> 
> - firefox://fxa-signin?signin=<token>&someQuery=<data>...
> - firefox://open-url?deep-link=https%253A...
> - firefox://anything-here-does-not-matter?deeplink=https%253A...
> 
> This reflects in the code, which is handling the last two cases combined and
> seems all over the place. And the URL style for the last two is not enforced
> .. you can actually replace open-url with anything. This is not strict
> enough.
> 
> I would like to ask for three things:
> 
> - Introduce a firefox://deep-link?url= style url
> - Enforce a firefox://open-url?url= style for the public API
> - Refactor this code to have three clear separate blocks or functions to
> handle firefox://fxa-signin, firefox://open-url and firefox://deep-link.

Stefan to be clear, the following deep links would be available from external apps:
firefox://fxa-signin, firefox://open-url

The following deep links would not be available from external apps:
firefox://deep-link

??
Flags: needinfo?(sarentz)

Comment 2

11 months ago
(In reply to Stefan Arentz [:st3fan] from comment #0)
> I'm not happy with how we now have three different things under the
> firefox:// scheme:
> 
> - firefox://fxa-signin?signin=<token>&someQuery=<data>...
> - firefox://open-url?deep-link=https%253A...
> - firefox://anything-here-does-not-matter?deeplink=https%253A...
> 
> This reflects in the code, which is handling the last two cases combined and
> seems all over the place. And the URL style for the last two is not enforced
> .. you can actually replace open-url with anything. This is not strict
> enough.
> 
> I would like to ask for three things:
> 
> - Introduce a firefox://deep-link?url= style url
> - Enforce a firefox://open-url?url= style for the public API
> - Refactor this code to have three clear separate blocks or functions to
> handle firefox://fxa-signin, firefox://open-url and firefox://deep-link.

Your proposal makes sense to me. Should this also handle existing deep linking capabilities? Currently, FxiOS in App Store can launch any url with firefox://?url=<url>.
(Assignee)

Comment 3

11 months ago
Created attachment 8877324 [details] [review]
PR
Attachment #8877324 - Flags: review?(sarentz)
(Assignee)

Updated

10 months ago
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1355636
(Reporter)

Updated

3 months ago
Flags: needinfo?(sarentz)
(Reporter)

Updated

3 months ago
Attachment #8877324 - Flags: review?(sarentz)
You need to log in before you can comment on or make changes to this bug.