Let other developers directly open pages in Firefox for iOS

RESOLVED FIXED

Status

()

Firefox for iOS
Browser
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: st3fan, Assigned: bkmunar)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

unspecified
All
iOS 7
dev-doc-needed
Dependency tree / graph

Firefox Tracking Flags

(fxios2.0+)

Details

Attachments

(7 attachments, 5 obsolete attachments)

(Reporter)

Description

3 years ago
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'.
This is actually something we also want for Android -- deep linking for Twitter. Let's make sure those things agree.
OS: Mac OS X → iOS 7
Hardware: x86 → All

Comment 2

3 years ago
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
Flags: needinfo?(sarentz)
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.
(Reporter)

Comment 4

3 years ago
(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)
Flags: needinfo?(sarentz)
tracking-fennec: --- → ?
tracking-fennec: ? → -
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
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.)
(Reporter)

Comment 7

3 years ago
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.
Assignee: nobody → bmunar
Status: NEW → ASSIGNED
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.
tracking-fennec: - → ?
tracking-fxios: --- → ?
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1109688
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1168131
(Reporter)

Updated

2 years ago
Summary: Find out how we can let other developers directly open pages in Firefox for iOS → Let other developers directly open pages in Firefox for iOS
(Assignee)

Comment 11

2 years ago
Created attachment 8623797 [details]
Currently working - here is my progress
(Assignee)

Comment 12

2 years ago
Created attachment 8623867 [details] [review]
PR (part 1)

Beginning of starter implementation for opening links in FFX-iOS
Attachment #8623867 - Flags: review?(sarentz)
(Reporter)

Updated

2 years ago
Attachment #8623867 - Flags: review?(sarentz) → review?(etoop)
(Reporter)

Updated

2 years ago
Attachment #8623867 - Flags: review?(sarentz)
Just a couple of small swift-ifications but otherwise good stuff :)
Comment on attachment 8623867 [details] [review]
PR (part 1)

LGTM
Attachment #8623867 - Flags: review?(etoop) → review+
(Reporter)

Comment 15

2 years ago
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:).
(Reporter)

Comment 16

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

Looks good, please merge.
Attachment #8623867 - Flags: review?(sarentz) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

2 years ago
mock upzz plzz!
Flags: needinfo?(randersen)
Flags: needinfo?(dhenein)
(Assignee)

Comment 18

2 years ago
Currently working on handling the UI for callbacks to Applications who open links in FFX-iOS and the client/app code for it!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 8625963 [details]
Back to App.png

Here is a quick mock of the back to app UI.
Flags: needinfo?(randersen)
(Assignee)

Comment 20

2 years ago
o0o0o0o0o0o0o0o mazin' mazing
Created attachment 8625974 [details]
Back to App.png
Attachment #8625963 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
tracking-fennec: ? → ---
tracking-fxios: ? → +
Created attachment 8626612 [details]
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.
Additionally, cap the label to 15 characters and use an ellipsis to truncate.
(Assignee)

Comment 24

2 years ago
ooooo byootiful, thanks robin!
Blocks: 1151532
(Assignee)

Comment 25

2 years ago
Created attachment 8628391 [details] [review]
PR (part 2)

just want the logic for handling the UI changes to be reviewed and refactored, etc.
Attachment #8628391 - Flags: review?(sarentz)
Attachment #8628391 - Flags: review?(rnewman)
Attachment #8628391 - Flags: feedback?(sarentz)
Attachment #8628391 - Flags: feedback?(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.
Attachment #8628391 - Flags: review?(sarentz)
Attachment #8628391 - Flags: review?(rnewman)
Created attachment 8628515 [details]
Back to App.png

updated with new caret asset example, more detail for text label.
Attachment #8625974 - Attachment is obsolete: true
Created attachment 8628516 [details]
Back to App Landscape.png

updated with new caret asset example, more detail for text label.
Attachment #8626612 - Attachment is obsolete: true
Created attachment 8628517 [details]
backCaret.zip

back caret assets.
: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)
Keywords: dev-doc-needed
Comment on attachment 8628391 [details] [review]
PR (part 2)

Feedback in the PR.
Attachment #8628391 - Flags: feedback?(rnewman) → feedback+
(Assignee)

Comment 32

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

revised code
Attachment #8628391 - Flags: feedback?(sarentz)
Attachment #8628391 - Flags: feedback?
Attachment #8628391 - Flags: feedback+
(Assignee)

Updated

2 years ago
Attachment #8628391 - Flags: feedback? → review?(rnewman)
Flags: needinfo?(dhenein)
(Assignee)

Comment 33

2 years ago
Created attachment 8629015 [details]
Code for 3rd party SDKs, please review! "OpenInChromeAndFirefoxController"
Attachment #8629015 - Flags: review?(sarentz)
Attachment #8629015 - Flags: review?(rnewman)
(Reporter)

Comment 34

2 years ago
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.
Attachment #8629015 - Flags: review?(sarentz) → review-
(Assignee)

Comment 35

2 years ago
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!
(Assignee)

Comment 36

2 years ago
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)
(Assignee)

Comment 37

2 years ago
Created attachment 8629459 [details]
3rd party code
Attachment #8629459 - Flags: review?(sarentz)
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?
Flags: needinfo?(randersen)
(Assignee)

Comment 39

2 years ago
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)
(Assignee)

Comment 40

2 years ago
Aka, if we don't use that class apple provides, we don't have access to that caret
: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.
Flags: needinfo?(randersen)
Attachment #8628391 - Flags: review?(rnewman) → review?(bnicholson)
Attachment #8623797 - Attachment is obsolete: true
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.
Attachment #8623867 - Attachment description: PR → PR (part 1)
Attachment #8628391 - Attachment description: PR - please ignore code signing and other random stuff i changed to run on iOS 8.4 (my phone hehe), and also ignore making button code → PR (part 2)
(Assignee)

Updated

2 years ago
Attachment #8629459 - Flags: review?(sarentz) → review?(bnicholson)
Blocks: 1180953
(Assignee)

Updated

2 years ago
Attachment #8629459 - Flags: feedback?(sarentz)
(Assignee)

Comment 43

2 years ago
Comment on attachment 8629459 [details]
3rd party code

https://github.com/bkmunar/OpenInFirefoxClient/pull/2
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
: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.
Created attachment 8630802 [details]
backCaret.zip

updated backCaret
Attachment #8628517 - Attachment is obsolete: true
(Assignee)

Comment 47

2 years ago
Comment on attachment 8629459 [details]
3rd party code

https://github.com/bkmunar/OpenInFirefoxClient/pull/3/files
(Reporter)

Comment 48

2 years ago
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.
Attachment #8629459 - Flags: feedback?(sarentz)
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?
Attachment #8629459 - Flags: review?(bnicholson) → review+
Comment on attachment 8628391 [details] [review]
PR (part 2)

Clearing review until new version is ready.
Attachment #8628391 - Flags: review?(bnicholson)
(Assignee)

Updated

2 years ago
Depends on: 1184591, 1184603
Attachment #8629015 - Flags: review?(rnewman)
(Assignee)

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
(Assignee)

Comment 51

2 years ago
Done for v1 for now, will file separate bugs for v1.1 and on
(Reporter)

Updated

2 years ago
tracking-fxios: + → 2.0+
You need to log in before you can comment on or make changes to this bug.