1391692 - Send an event when the user bookmarks on the new tab page

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect)

Description

[Simon Basca [:SimonB]]

Build: 918e20ecd
Device: iPad Air 2
iOS: 10.3.3

Steps to reproduce:
1. Open a new tab
2. Long tap on a Top Site or Highlight 
3. Tap 'Bookmark'
4. Check the Leanplum dashboard

Other valid steps:
1. Go to goole.com
2. Tap the 'Share' button
3. Tap on the 'Fennec' extension
4. Tap 'Add to Bookmarks'
5. Check the Leanplumb dashboard

Actual result:
- The event "E_Saved_Bookmark" is not triggered.

Expected results;
- The "E_Saved_Bookmark" should be triggered when bookmarking from the Activity Stream context menu or from the Fennec extension from the Share menu.

Comment 1

[Bryan Munar (:bkmunar)]

Attachment: PR

tested and works, only the extension bookmark leanplum events get sent when the application becomes active/foregrounded

Comment 2

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

Comment on attachment 8899959 [details] [diff] [review]
PR

The AS context menu part is fine. But I would like to see a separate bug for the Share Extension.

I am also not willing to approve the current implementation for the share extension. It is too hacky. We need a better way.

I also think that maybe using the share extension warrants its own set of Leanplum events. Since it is really about sharing to Firefox (bookmarks and reading list).

Comment 3

[Simon Basca [:SimonB]]

Created separate issue for the Firefox extension (Bug 1393060).

Comment 4

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

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

This patch sends a .saveBookmark event when the user hits Add Bookmark from either the top site or a highlight context menu.

This patch does not make an effort to integrate these events in the Share extention. That is out of scope for the 8.3 release.

Comment 5

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

Landed on master, uplifted to v8.x and v9.x

Comment 6

[Simon Basca [:SimonB]]

Verifying as fix on master b06b848a9f74.

Comment 7

[Simon Basca [:SimonB]]

Verifying as fix on beta 8.3(5817).