1385853 - [LeanPlum] - Default browser is still triggered if app was set as default from another app

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Bogdan Surd, QA [:bsurd, NI]]

Device:
 - LG G4 (Android 5.1);
 - Huawei MediaPad M2 (Android 5.1.1;

Steps to reproduce:
1. Open Gmail and from any mail open and link;
2. When prompted to select a browser select Fennec and then tap on always (or check  use as default browser and tap on ok);
3. Open Fennec and trigger any Default Browser action.

Expected result:
 None of the Default Browser triggers should be triggered since Fennec is set as the Default Browser

Actual result:
 If Fennec is set as the Default Browser when opening up a link from another app the trigger still occurs.

Notes:
 - When setting the app as the default browser from the settings everything works as expected.
 - The LG G4 with an android version of 5.1 does not have the option to set the Default Browser from the Settings menu.

Comment 1

[Max Liu [:maliu]]

[Tracking Requested - why for this release]:

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

My understanding is MMA SDK integration is planned for 56, changing the tracking nom from 57 to 56.

Comment 3

[Gerry Chang [:gchang]]

Track 56+ as MMA SDK is enabled in 56.

Comment 4

[Nevin Chen(Not active on Bugzilla)]

We check if Fennec is set as default browser when we launch Fennec app.
That means if the default browser setting changes while the app is still running, we'll still think Fennec is not set as default.
This is a design limitation cause we shouldn't check the setting too often.
I think this bug should be marked as invalid and won't fix. thus shouldn't track for 56+

Comment 5

[Wesly Huang (EPM)]

It's a good catch but a design limitation currently as Nevin said. Need a discussion with Product(maybe UX as well) to assess its priority. (personally prefer to fix it as P2, if feasible).

@Joe, how do you think?

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

currently we only promote setting default browser using dialog at app start so this bug seems to be avoided by our current campaign setup. We need to pay attention to this once we start using banner messages.

Comment 7

[Jean Collings]

There are other triggers we are using to get users to change their default browser to Firefox but the dialog at app start is the most successful one so far - agree, let's keep an eye on this when we start using banners.

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Please remove 56 tracking since I don't know when I will have time to fix it.

Comment 9

[Wesly Huang (EPM)]

(In reply to Gerry Chang [:gchang] from comment #3)
> Track 56+ as MMA SDK is enabled in 56.

Hi Gerry, per comment 6 and comment 7 this is not blocking 56 (the "banner" feature is currently in the backlog, not yet implemented), would you help remove the tracking flag accordingly? Thanks.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

[triage] Bulk edit: product thinks leanplum is high priority: P1 rank 1.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Product team would like this fixed in 60, tracked as blocking to ensure it's closely monitored.

Comment 12

[:sdaswani]

Jean, per your comment here https://bugzilla.mozilla.org/show_bug.cgi?id=1385853#c7, what exactly do we need to do under this ticket? I'm not sure I understand the ask.

Comment 13

[Jean Collings]

The main concern here is that we will show a "hey change your browser settings so that Firefox is your default browser" message to users who already have Firefox as a default. 

How do we make sure that we don't show this type of message to users who already set Firefox as default?

Comment 14

[:sdaswani]

Understood Jean, we will work on it for 60.

Comment 15

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug_1385853_Force_LeanPlum_resync_if_default_browser_changed.patch

I investigated this issue and found the following:
- To make sure we won't show the "Default Browser" banner if Fennec was already set as default (while the app was running) we need to recheck and sync this bool value everytime the app shows on the screen (like after coming back to Fennec from System Settings)
- Unfortunately LP does not allow by default rechecking the conditions to decide message's opportunity as per https://support.leanplum.com/hc/en-us/articles/115003470566-How-and-when-are-targets-evaluated- 
"For In-app messages, targets are evaluated at the start of a new session (Leanplum.start). If the user fits the target criteria at app start, the message will be synced to their device with all appropriate triggers and other settings.
But, if the user’s relevant segment criteria changes during that session, the user won’t be re-evaluated as a target till the start of their next session."

Hence for whenever we detect a change in default browser's status I see three possible approaches to update the attributes on server and get the updated list of messages:
1) Do a local modification of LeanPlum SDK files so that we can inject ourselfs to check if a message should be shown or not.
We could check if the message in process of being displayed has the action we know it's used for setting the default browser and stop the display process.
While this would work at runtime without needing to restart LeanPlum it does bring the inherent costs of modifying a 3rd party sdk, using hard-coded values and will add a performance penalty for checking everytime, every message if it needs to be displayed or not.

2) - Resync with LeanPlum server without restarting LP
        Leanplum.setUserAttributes(<new default browser attibute>);
        Leanplum.forceContentUpdate();
is not possible because even in this case LeanPlum will not get a new updated list of messages to work with.

    - Actually restart LeanPlum to force syncing of the new attributes and download an updated list of messages.
I ended up using this approach as I found it to be the least intrusive and the least expensive method.

Comment 16

[:sdaswani]

Petru thanks for the submission here. Upon understand the options to fix it, I am wary to actually fix it, and I go back to what Nevin said here: https://bugzilla.mozilla.org/show_bug.cgi?id=1385853#c4

Jean, while this fix may work, it adds some odd processing to catch what may not happen much. Restarting LP in session seems heavy duty and risky.

Can we talk to Evan about this Jean and see what he recommends?

Comment 17

[Jean Collings]

Sounds good.

Comment 18

[:sdaswani]

Sushee e-mailed LP about this.

Comment 19

[Julien Cristau [:jcristau]]

Removing blocking flag for 60 per whiteboard update.

Comment 20

[:sdaswani]

In my discussion with LP, they say we just need to disable the magnet to make sure that the user's properties are re-evaluated at startup.

Comment 21

[Petru-Mugurel Lingurar [:petru]]

Then we are left with what Nevin said in https://bugzilla.mozilla.org/show_bug.cgi?id=1385853#c4

>> We check if Fennec is set as default browser when we launch Fennec app.
>> That means if the default browser setting changes while the app is still running, we'll still think Fennec is not set as default.
>> This is a design limitation cause we shouldn't check the setting too often.
>> I think this bug should be marked as invalid and won't fix. thus shouldn't track for 56+

If the LP campaign showing this message is set to only show it when the app starts (not resumed) the user should not be affected by the issue described in this ticket.

Comment 22

[:sdaswani]

That makes sense Petru, thanks for the clarification. I will talk to the Product folks about this.

Comment 23

[Cherry]

Will have team test to see if this is a non-issue.

Comment 24

[kplam]

Hey Team - 

I was able to reproduce the steps listed above, and the event trigger is working correctly. 

The results from testing on a Google Pixel: Once a user sets Fx as their default browser, they will not receive messaging to set Fx as their default browser

Comment 25

[:sdaswani]

Great Kat, I'll close this now. Thanks for your help everyone!