1179067 - Add "View later" as a Send extension

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Richard Newman [:rnewman]]

Right now you can add to bookmarks or add to reading list… but not open the page in Firefox. Let's fix that.

I propose for v1 making the URL and Firefox icon clickable. We could also add a row. Thoughts?

Comment 1

[Richard Newman [:rnewman]]

I'm hacking this now. I also think we should add a line to "Open later", which gets us close to tab queues.

Comment 2

[Richard Newman [:rnewman]]

Attachment: Pull req.

Comment 3

[Richard Newman [:rnewman]]

For the record: the hack I wanted to use to launch Firefox from the extension doesn't work in 8.3 or later, so… let's morph!

(Leaving the needinfo for Robin so she's forced to clear it. Heh.)

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Watch the scope-creep

Comment 5

[Richard Newman [:rnewman]]

Over to Robin for icon and verb. CC antlam for verb alignment.

Comment 6

[Richard Newman [:rnewman]]

Scope can't creep on a feature that was entirely built in two hours :)

Comment 7

[Richard Newman [:rnewman]]

Blocking Bug 1168395 'cos this adds strings.

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8628057 [details] [review]
Pull req.

Only outstanding TODO is the icon, which Robin is providing.

Comment 9

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

Why is this in the Share extension and not a separate Action next to "Send Tab"? My gut feeling says that makes more sense.

Comment 10

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

Tab counter is not correctly incremented.

1) Open firefox, open a single tab with a random site
2) Switch to Safari
3) Queue 3 sites
4) Switch back to Firefox

Expected: Tab counter is set to 4

Actual: Tab counter is set to 2

Comment 11

[Darrin Henein [:darrin]]

(In reply to Stefan Arentz [:st3fan] from comment #9)
> Why is this in the Share extension and not a separate Action next to "Send
> Tab"? My gut feeling says that makes more sense.

I tend to agree. This would be a nice one-tap interaction, and we wouldn't even need UI (just label action "View Later" with firefox icon)...

Comment 12

[Darrin Henein [:darrin]]

Comment on attachment 8628057 [details] [review]
Pull req.

I'm going to propose we delay landing just to get it in, and rather get it right:

* I don't want to combine "permanent" save actions (bookmark/reading list) with what is basically "open in background"

* this would be much better as a one-touch interaction, and probably doesn't need any UI

Comment 13

[Richard Newman [:rnewman]]

Dependent code landed, so the frontend implementation just needs to call

        profile.queue.addToQueue(item).value

a la

https://github.com/mozilla/firefox-ios/commit/9df6adc7463ee039335e1a5aba0ab38b00b9fe4f#diff-81672afe010f3069dcc0c8a817355225R129

and everything else will Just Work®.

Comment 14

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

Not entirely sure why this is failing for me:

2015-07-06 16:35:37.442 ViewLater[13150:1395728] Failed to inherit CoreMedia permissions from 13137: (null)
2015-07-06 16:35:38.015 [Error] [Profile.swift:42] init(profile:): Unable to find the shared container. Defaulting profile location to ~/Documents instead.
2015-07-06 16:35:38.018 [Debug] [BrowserDB.swift:73] init(filename:secretKey:files:): Initializing BrowserDB.
2015-07-06 16:35:38.018 [Debug] [BrowserDB.swift:137] createOrUpdate: Create or update tableList version 1.
2015-07-06 16:35:38.021 [Debug] [BrowserDB.swift:88] createTable(_:table:): Try create tableList version 1
2015-07-06 16:35:38.021 [Debug] [SwiftData.swift:125] transaction: Op in transaction succeeded. Committing.
2015-07-06 16:35:38.022 [Info] [BrowserTable.swift:67] init(): SQLite version: 3.8.8.3 (3008008).
2015-07-06 16:35:38.022 [Debug] [BrowserDB.swift:137] createOrUpdate: Create or update BROWSER version 5.
2015-07-06 16:35:38.024 [Debug] [BrowserDB.swift:346] tablesExist: 0 tables exist. Expected 5
2015-07-06 16:35:38.024 [Debug] [BrowserDB.swift:88] createTable(_:table:): Try create BROWSER version 5
2015-07-06 16:35:38.024 [Debug] [BrowserTable.swift:225] create(_:version:): Creating 10 tables, views, and indices.
2015-07-06 16:35:38.026 [Error] [SwiftData.swift:400] executeChange(_:withArgs:): SQL error: SQL error or missing database no such table: main.favicons for SQL CREATE VIEW IF NOT EXISTS view_favicons_widest AS SELECT favicon_sites.siteID AS siteID, favicons.id AS iconID, favicons.url AS iconURL, favicons.date AS iconDate, favicons.type AS iconType, MAX(favicons.width) AS iconWidth FROM favicon_sites, favicons WHERE favicon_sites.faviconID = favicons.id GROUP BY siteID .
2015-07-06 16:35:38.026 [Error] [BrowserTable.swift:73] run(_:sql:args:): Error running SQL in BrowserTable. Optional("SQL error or missing database no such table: main.favicons")
2015-07-06 16:35:38.026 [Error] [BrowserTable.swift:74] run(_:sql:args:): SQL was CREATE VIEW IF NOT EXISTS view_favicons_widest AS SELECT favicon_sites.siteID AS siteID, favicons.id AS iconID, favicons.url AS iconURL, favicons.date AS iconDate, favicons.type AS iconType, MAX(favicons.width) AS iconWidth FROM favicon_sites, favicons WHERE favicon_sites.faviconID = favicons.id GROUP BY siteID 
2015-07-06 16:35:38.026 [Debug] [BrowserDB.swift:91] createTable(_:table:): Creation failed.
2015-07-06 16:35:38.026 [Debug] [SwiftData.swift:132] transaction: Op in transaction failed. Rolling back.
2015-07-06 16:35:38.026 [Debug] [BrowserDB.swift:190] createOrUpdate: Couldn't create or update BROWSER.
2015-07-06 16:35:38.026 [Debug] [BrowserDB.swift:191] createOrUpdate: Attempting to move browser.db to another location.
2015-07-06 16:35:38.027 [Debug] [BrowserDB.swift:88] createTable(_:table:): Try create BROWSER version 5
2015-07-06 16:35:38.027 [Debug] [BrowserTable.swift:225] create(_:version:): Creating 10 tables, views, and indices.
2015-07-06 16:35:38.029 [Error] [SwiftData.swift:400] executeChange(_:withArgs:): SQL error: SQL error or missing database no such table: main.favicons for SQL CREATE VIEW IF NOT EXISTS view_favicons_widest AS SELECT favicon_sites.siteID AS siteID, favicons.id AS iconID, favicons.url AS iconURL, favicons.date AS iconDate, favicons.type AS iconType, MAX(favicons.width) AS iconWidth FROM favicon_sites, favicons WHERE favicon_sites.faviconID = favicons.id GROUP BY siteID .
2015-07-06 16:35:38.029 [Error] [BrowserTable.swift:73] run(_:sql:args:): Error running SQL in BrowserTable. Optional("SQL error or missing database no such table: main.favicons")
2015-07-06 16:35:38.029 [Error] [BrowserTable.swift:74] run(_:sql:args:): SQL was CREATE VIEW IF NOT EXISTS view_favicons_widest AS SELECT favicon_sites.siteID AS siteID, favicons.id AS iconID, favicons.url AS iconURL, favicons.date AS iconDate, favicons.type AS iconType, MAX(favicons.width) AS iconWidth FROM favicon_sites, favicons WHERE favicon_sites.faviconID = favicons.id GROUP BY siteID 
2015-07-06 16:35:38.029 [Debug] [BrowserDB.swift:91] createTable(_:table:): Creation failed.
2015-07-06 16:35:38.029 [Debug] [SwiftData.swift:132] transaction: Op in transaction failed. Rolling back.
2015-07-06 16:35:46.164 [Error] [SwiftData.swift:400] executeChange(_:withArgs:): SQL error: SQL error or missing database no such table: queue for SQL INSERT OR IGNORE INTO queue (url, title) VALUES (?, ?).
2015-07-06 16:35:46.164 [Debug] [DeferredDBOperation.swift:91] main(): Modified rows: 0.
2015-07-06 16:35:52.177 ViewLater[13150:1395735] host connection <NSXPCConnection: 0x7a726e50> connection from pid 13137 invalidated


"SQL error or missing database no such table: queue"


Maybe something is missing from the original PR.

Comment 15

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

This may be due to missing entitlements so that the app cannot access the shared container.

Comment 16

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

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/680

This patch adds a new View Later action extension.

Comment 17

[Stephan Leroux [:sleroux]]

Comment on attachment 8630515 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/680

Code looks good! This feature rocks btw. Maybe not for v1 but I feel like we need some UI feedback when we go back into the app when restoring view later tabs. Like a toast that shows "opened up 3 viewed later tabs" or something.

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

Was there any discussion about the fact that Android uses "Open Now | Later" and iOS uses "View Later" ? Should we try to be consistent with features across platforms?

I am also a little worried that the UX around this feature is not completely baked yet and it's adding some risk.

Comment 19

[Richard Newman [:rnewman]]

Robin and Darrin talked this through quite a bit, and settled on 'view' as the verb.

I started with "Open Later" in my proto, and UX definitely spent the time to make a firm initial decision.

My personal opinion: I don't think cross-platform consistency is super valuable in this case; so much already differs that there's no shared framing, and probably not a ton of overlapping users. I trust Darrin and Robin to pick something they think will make sense to the iOS user base (and also to change their minds later if the evidence is there!).


Re risk: the feature is really very simple (much simpler than the equivalent on Android), so the only risk I see is around strings. Worst case we're in string freeze and we just punt this to 1.1 by disabling the extension. We can weigh that option against negative or positive feedback from the experience during TestFlight.

ni Robin to see if she has anything to add.

Comment 20

[Robin Andersen [:tecgirl]]

I still think "View" is appropriate, but I do want a toast to let the user know it was successful.  I would like to at least implement the toasts I designed for "Send Tab". We can keep those strings as simple as possible. 

http://invis.io/DE3BHFHMW (Success)
http://invis.io/293LMTPC3 (Please try again)

Comment 21

[Richard Newman [:rnewman]]

-> Bug 1184555