If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Today Widget – New Tab & New Private Tab

RESOLVED FIXED

Status

()

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

People

(Reporter: jhugman, Assigned: jhugman)

Tracking

(Blocks: 1 bug)

unspecified
Other
iOS

Firefox Tracking Flags

(fxios4.0+)

Details

Attachments

(3 attachments)

(Assignee)

Description

2 years ago
Add a today widget, with new tab and new private tab buttons.
(Assignee)

Updated

2 years ago
Assignee: nobody → jhugman
Blocks: 1186573
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
I need to update the firefox:// URL scheme (because opening the containing app can only be done via a call to openURL).

Right now, we can only open URLs in a non-private tab, by passing a url query parameter.

I propose a second query parameter, so a new private tab is:

firefox://?private=true
firefox://?private=true&url=http://mozilla.com (modulo appropriately URL encoding)

Second: I propose we change the default behaviour when no url is present to be changed from opening the app to the current tab to opening a new tab.

Both open a new tab:

firefox://
firefox://?

:sleroux, :st3fan, what say you?
Flags: needinfo?(sleroux)
> I propose a second query parameter, so a new private tab is:
> firefox://?private=true
> firefox://?private=true&url=http://mozilla.com (modulo appropriately URL encoding)

Make sense. I think the only thing we'd want to do is formally document that it means for the parameter to be absent (private=false) somewhere. Not sure if we have documentation about this anywhere.

> Second: I propose we change the default behaviour when no url is present to be changed from opening the app to the current tab to opening a new tab.

Do we know if anyone is using this API already? With apps like Outlook/Airmail supporting Firefox I wonder if there is already an assumption around how this API works. I'd just be worried about breaking API changes. Also, if we wanted to expose an API for just opening the app I feel like we hamstring ourselves here unless we add a new parameter like 'justopen' which seems a bit jank. An alternative could be passing in about:blank into the URL parameter to open a blank new tab since that would align with the existing usage of opening a tab..
Flags: needinfo?(sleroux)
(Assignee)

Comment 3

2 years ago
> Do we know if anyone is using this API already? With apps like Outlook/Airmail supporting Firefox I wonder if there is already an assumption around how this API works.
The only thing I could find was :bkmunar's docs from Open In Firefox. Otherwise, the only documentation is the code itself. https://github.com/mozilla/firefox-ios-open-in-client#the-custom-firefox-url

> Also, if we wanted to expose an API for just opening the app I feel like we hamstring ourselves here unless we add a new parameter like 'justopen' which seems a bit jank.
In the absence of error handling, 'just open' the app is the default error behaviour. I don't think we should be advertising this as part of the API.

Digging further in (lots of duplicated code due to #available(iOS 9, *)) checks, it seems that the default behaviour is to open a new tab in whatever mode (private or not) the tab tray is currently in. 

> An alternative could be passing in about:blank into the URL parameter to open a blank new tab since that would align with the existing usage of opening a tab..
I considered this, though that does prevent us adding 'new tab' behaviour, such as opening a default new tab page, or going straight to the tab tray.
Flags: needinfo?(sleroux)
(Assignee)

Comment 4

2 years ago
Please may I have some assets for this? I'm happy to use recycled assets, though you may not be :)

 – New Tab, .Normal & .Highlighted states
 - New Private Tab, .Normal and .Hightlighted states.

Thank you muchly.
Flags: needinfo?(randersen)
> In the absence of error handling, 'just open' the app is the default error behaviour. I don't think we should be advertising this as part of the API.

True. The more I think about it, the more I realize that we probably don't ever want to 'just open' the tab since the schemes are really meant for invoking an action in the other app. I don't see why another app would want to open FF and do nothing.

> Second: I propose we change the default behaviour when no url is present to be changed from opening the app to the current tab to opening a new tab.

By using train of thought in the previous example this makes more sense. Other applications probably don't want to do nothing. They most likely will want to open a new tab for themselves and perform some kind of action on it.
Flags: needinfo?(sleroux)
Created attachment 8728056 [details]
TodayWidgetAssets.zip
Flags: needinfo?(randersen)
(Assignee)

Comment 7

2 years ago
Created attachment 8729641 [details] [review]
Pull request

I've got everything working, though would like some feedback on the rebase which caused me some issues, and some SnapKit layout problems I'm having (Private Tab button).
Attachment #8729641 - Flags: feedback?(sleroux)
Comment on attachment 8729641 [details] [review]
Pull request

Left feedback on the PR.
Attachment #8729641 - Flags: feedback?(sleroux) → feedback+
(Assignee)

Updated

2 years ago
tracking-fxios: --- → 4.0+
(Assignee)

Comment 9

2 years ago
Created attachment 8736845 [details]
iPhone screenshot
Attachment #8736845 - Flags: ui-review?(randersen)
(Assignee)

Comment 10

2 years ago
Comment on attachment 8729641 [details] [review]
Pull request

I added the height using 2x the button size.
Attachment #8729641 - Flags: review?(sleroux)
Comment on attachment 8736845 [details]
iPhone screenshot

Looks good! Can we reduce the label size to 14pt and give it a bit of space from the icon? Say, 10px?
(Assignee)

Updated

2 years ago
Attachment #8729641 - Flags: review?(etoop)
Comment on attachment 8729641 [details] [review]
Pull request

Looks good. Left a couple of minor suggestions in the PR.

I wasn't able to test this as I couldn't build it in Xcode 7.3 (compiler issues)
Attachment #8729641 - Flags: review?(etoop) → review+
Attachment #8736845 - Flags: ui-review?(randersen) → ui-review+
(Assignee)

Comment 13

2 years ago
Nits addressed. 

Merged.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Attachment #8729641 - Flags: review?(sleroux)
Design at https://mozilla.invisionapp.com/share/9J5Z5F2WS#/screens
You need to log in before you can comment on or make changes to this bug.