1109684 - Let other developers directly open pages in Firefox for iOS

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Chrome exposes a URL scheme that can be used to open Chrome and make it load a page. I think we want to support something similar so that other developers can include a 'Open web pages in Firefox' option to their application.

Maybe we want to extend this a bit more and let other apps do things like:

* Open in Firefox
* Open in Firefox in Reading Mode

I don't think this is a replacement for the Action and Share extensions that we also have. I think it complements those. Many apps have shortcut buttons for actions like 'open in browser' or 'add to my preferred reading list' and that is exactly why we would expose this 'API'.

Comment 1

[Richard Newman [:rnewman]]

This is actually something we also want for Android -- deep linking for Twitter. Let's make sure those things agree.

Comment 2

[Sachin Palewar]

To take this further, I think first we need to finalise the URI schemes Firefox will handle. Chrome handles following:

googlechrome for http
googlechromes for https
googlechrome-x-callback for callbacks

May be we need another for open in Firefox in Reading mode?

Will these be ok?

firefox
firefoxs
firefox-x-callback
firefox-reader

Comment 3

[Richard Newman [:rnewman]]

Those sound good to me, but:

* We support multiple release channels on Android. We might well do so on iOS. The choice we make here has to allow for that -- can multiple apps consume the same URL schemes? If not, are we happy with 'fennecs:'?

* This is a public API, and once settled it's harder to change than other decisions. It would be worth starting a discussion on mobile-firefox-dev to get additional opinions before committing.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

(In reply to Richard Newman [:rnewman] from comment #3)
> Those sound good to me, but:
> 
> * We support multiple release channels on Android. We might well do so on
> iOS. The choice we make here has to allow for that -- can multiple apps
> consume the same URL schemes? If not, are we happy with 'fennecs:'?

I don't think they can. It is unclear what happens if multiple apps implement the same scheme. Probably random or some undefined order.

In reality we will not be able to push a Beta or Nightly channel to the App Store anyway. Apple does not allow it. This is why there also is no Chrome Beta on the App Store. So the audience for this specific feature is *extremely* limited.

So I would suggest that we do different schemes and that we create a small 'SDK' that third party apps can include. This SDK will basically wrap UIApplication.canOpenURL()/openURL() and by default just finds and delegates to the most 'stable' version of Firefox installed, with the option to specify a specific version if needed. (But I doubt third party apps would use that since it makes no sense on the App Store. It would be nice for our own testing and experiments though)

Comment 5

[Wesley Johnston (:wesj)]

I'm a bit surprised this is tracking-... but Chrome has a pretty simple library that devs can use to do this (on their side, this doesn't include the code on our side). We should fork it:

https://github.com/GoogleChrome/OpenInChrome/blob/master/OpenInChromeController.m

Comment 6

[Richard Newman [:rnewman]]

I think it's tracking- 'cos we wouldn't block ship on it. IMO we definitely want to do this, but we shouldn't rush something that we can't change later -- so let's aim for v1.1/v2.0.

(If we can get a good, solid solution done sooner without slipping other goals, then great, of course.)

Comment 7

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I would also like to see this included sooner than later.

But I think think we should only ship this if we can also convince a bunch of third-party apps to use it. Examples that come to mind: Twitterific, Tweetbot, Reeder. These all support some form of 'open in $BROWSER_OF_CHOICE' if I remember correctly.

I can send out some pings to developers and see what they think.

Comment 8

[Brian Nicholson (:bnicholson)]

There's more discussion about this recently, and we decided tracking- back when June 2 was the expected ship date. Renominating to be sure this doesn't get overlooked.

Comment 11

[Bryan Munar (:bkmunar)]

Attachment: Currently working - here is my progress

Comment 12

[Bryan Munar (:bkmunar)]

Attachment: PR (part 1)

Beginning of starter implementation for opening links in FFX-iOS

Comment 13

[Emily Toop (:fluffyemily)]

Just a couple of small swift-ifications but otherwise good stuff :)

Comment 14

[Emily Toop (:fluffyemily)]

Comment on attachment 8623867 [details] [review]
PR (part 1)

LGTM

Comment 15

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8623867 [details] [review]
PR (part 1)

I don't want to + this just yet because there are some issues in the PR that need to be addressed. Most important is using NSURLComponents to parse the incoming URL and to not use force unwrapping on NSURL(string:).

Comment 16

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8623867 [details] [review]
PR (part 1)

Looks good, please merge.

Comment 17

[Bryan Munar (:bkmunar)]

mock upzz plzz!

Comment 18

[Bryan Munar (:bkmunar)]

Currently working on handling the UI for callbacks to Applications who open links in FFX-iOS and the client/app code for it!

Comment 19

[Robin Andersen [:tecgirl]]

Attachment: Back to App.png

Here is a quick mock of the back to app UI.

Comment 20

[Bryan Munar (:bkmunar)]

o0o0o0o0o0o0o0o mazin' mazing

Comment 21

[Robin Andersen [:tecgirl]]

Attachment: Back to App.png

Comment 22

[Robin Andersen [:tecgirl]]

Attachment: Back to App Landscape.png

For landscape, it's going to be a bit tricky, but (I think/hope) we can do it. 

On load, display the caret and label—but disable the rest of the nav.
If the user taps on a link within that view, re-enable the nav and disable the caret and label.

Comment 23

[Robin Andersen [:tecgirl]]

Additionally, cap the label to 15 characters and use an ellipsis to truncate.

Comment 24

[Bryan Munar (:bkmunar)]

ooooo byootiful, thanks robin!

Comment 25

[Bryan Munar (:bkmunar)]

Attachment: PR (part 2)

just want the logic for handling the UI changes to be reviewed and refactored, etc.

Comment 26

[Richard Newman [:rnewman]]

Comment on attachment 8628391 [details] [review]
PR (part 2)

I doubt Stefan will get to this today (Canada day!). Let's see if I manage it.

Comment 27

[Robin Andersen [:tecgirl]]

Attachment: Back to App.png

updated with new caret asset example, more detail for text label.

Comment 28

[Robin Andersen [:tecgirl]]

Attachment: Back to App Landscape.png

updated with new caret asset example, more detail for text label.

Comment 29

[Robin Andersen [:tecgirl]]

Attachment: backCaret.zip

back caret assets.

Comment 30

[Robin Andersen [:tecgirl]]

:bkmunar you nipped out before I had a chance to answer so I'm leaving this here… 

15:51 <bkmunar> 
i know you're on pto sigh, but wanna give me a brief talk about how to include assets with varying sizes? and/or point me to an example in the code? ahha sorry again >.<

As far as I know, you use the UIImageView class.

Example:
In Client/Frontend/Reader/ReaderModeStyleViewController.swift
157    let brightnessMinImageView = UIImageView(image: UIImage(named: "brightnessMin"))
158    brightnessRow.addSubview(brightnessMinImageView) 

The image is located in ReaderSettings.xcassets

Xcode now supports Asset Catalogs, so all you have to do is drag them here and it will automatically render a .json with the calls for the varying sizes. At first they will display below and you'll see three boxes [@1x][@2x][@3x] to drag them in. Make sure they are assigned "Universal" and you should be good to go!

To make a new Asset Catalog, just find another .xcassets file and click the [+] on the bottom left of the asset list column. (at least, this is the easiest way I can find to do it)

Comment 31

[Richard Newman [:rnewman]]

Comment on attachment 8628391 [details] [review]
PR (part 2)

Feedback in the PR.

Comment 32

[Bryan Munar (:bkmunar)]

Comment on attachment 8628391 [details] [review]
PR (part 2)

revised code

Comment 33

[Bryan Munar (:bkmunar)]

Attachment: Code for 3rd party SDKs, please review! "OpenInChromeAndFirefoxController"

Comment 34

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8629015 [details]
Code for 3rd party SDKs, please review! "OpenInChromeAndFirefoxController"

I don't like this for several reasons.

First this code starts with "Copyright 2012-2014, Google Inc". It has their license on top. Which is really odd.

Second, by supporting both Chrome and Firefox we now have become the maintainers of the Chrome code too. This is not what we want.

My suggestion is to create a new project from scratch with three files in it:

 * OpenInFirefox.swift
 * OpenInFirefox.h
 * OpenInFirefox.m

Both implementations do the same. One is for Swift projects, the other for Objective-C projects.

The reason for providing both a Swift and Objective-C implementation is to make it simpler for developers to include their preferred implementation language.

Comment 35

[Bryan Munar (:bkmunar)]

St3fan: ok! I have the code for that too which I shall upload; My assumption to what Richard meant by adding Firefox support for the chrome code too was to fork it and add it in? So I did that and have my custom files too that are what you specified above. Thanks!

Comment 36

[Bryan Munar (:bkmunar)]

For reference, look at my OpenInFirefoxClient project on github, that has the code there (but needs to be updated a bit and will get to)

Comment 37

[Bryan Munar (:bkmunar)]

Attachment: 3rd party code

Comment 38

[Darrin Henein [:darrin]]

Quick question – is there a reason we use a custom back caret vs. the system supplied one? I don't think we use a custom caret anywhere else, do we?

Comment 39

[Bryan Munar (:bkmunar)]

Darrin, it's because urlBar doesn't use the one apple provides (navigation toolbar or something?) which provides the back Caret for you, so we need to make a custom one (I believe)

Comment 40

[Bryan Munar (:bkmunar)]

Aka, if we don't use that class apple provides, we don't have access to that caret

Comment 41

[Robin Andersen [:tecgirl]]

:darrin  - I prefer the native asset but Bryan ran into the above issue. I at least rounded it out a little to match the rest of the UI.

Comment 42

[Brian Nicholson (:bnicholson)]

Comment on attachment 8623867 [details] [review]
PR (part 1)

Just for future reference, once the PR is merged and the bug is marked as RESOLVED/FIXED, it's best to open a new bug to address any follow-ups. Reopening the same bug for multiple PRs adds clutter and is harder to follow.

Comment 43

[Bryan Munar (:bkmunar)]

Comment on attachment 8629459 [details]
3rd party code

https://github.com/bkmunar/OpenInFirefoxClient/pull/2

Comment 44

[Brian Nicholson (:bnicholson)]

For the third party code, I created a new mozilla repository that we can move this to once r+'d: https://github.com/mozilla/firefox-ios-open-in-client

Comment 45

[Robin Andersen [:tecgirl]]

:bkmunar when in landscape, the navigation is disabled. The back caret and label display. If the user taps anything in the webview, the label disappears and becomes a back arrow. If the user goes back, the caret and label is displayed *and* the forward arrow. I'm making a new back caret that is essentially the back arrow without the line. If this doesn't make sense let me know and we can hop on a vidyo chat.

Comment 46

[Robin Andersen [:tecgirl]]

Attachment: backCaret.zip

updated backCaret

Comment 47

[Bryan Munar (:bkmunar)]

Comment on attachment 8629459 [details]
3rd party code

https://github.com/bkmunar/OpenInFirefoxClient/pull/3/files

Comment 48

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8629459 [details]
3rd party code

I removed myself from feedback to clear my queue. But if this patch is finished and you want a second pair of eyes on it, please add me back.

Comment 49

[Brian Nicholson (:bnicholson)]

Comment on attachment 8629459 [details]
3rd party code

3rd party PR looks OK to me. Can you squash all of these into a single commit before landing?

Comment 50

[Brian Nicholson (:bnicholson)]

Comment on attachment 8628391 [details] [review]
PR (part 2)

Clearing review until new version is ready.

Comment 51

[Bryan Munar (:bkmunar)]

Done for v1 for now, will file separate bugs for v1.1 and on