1519418 - MMA deeplinks do not work on debug variants

Status: RESOLVED FIXED

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

Description

[Vlad Baicu]

MMA deeplinks currently do not work on debug variants without overriding a check in https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/LauncherActivity.java#162.

This is due to the debug id not being stored in shared preferences -https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java#230.

Comment 1

[Vlad Baicu]

Attachment: Bug 1519418 - Retrieve LP debug id for local uid on debug builds. r=sdaswani

On debug builds whenever we would attempt to retrieve the value of localUid it
would be null because the LP debug id is never persisted.

Comment 2

[:sdaswani]

Daniel also any concerns with this change?

Comment 3

[Daniel Veditz [:dveditz]]

no

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/259e581af979
Retrieve LP debug id for local uid on debug builds. r=petru

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/259e581af979

Comment 6

[Vlad Baicu]

Reopening this as there is a problem with the matching of the local uid and the one received in the intent due to line separators.

Comment 7

[Vlad Baicu]

Attachment: Bug 1519418 - Replace line separators from the uid parameter received from the deeplink intent. r=sdaswani

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/348428783522
Replace line separators from the uid parameter received from the deeplink intent. r=sdaswani

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/348428783522

Comment 10

[Vlad Baicu]

Comment on attachment 9039863 [details]
Bug 1519418 - Replace line separators from the uid parameter received from the deeplink intent. r=sdaswani

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1519418 previous patch

User impact if declined

Patch from bug 1515651 will not work, because 259e581af979 introduces a regression for it. 259e581af979 landed in beta, whereas this patch did not.

Is this code covered by automated tests?

Unknown

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

Yes

If yes, steps to reproduce

Once uplifted, attempt to send a push notification with the following action: firefox://open?url=https://www.mozilla.org/ro/firefox/mobile/&uid={{User ID}}, if the notification is opened, the problem is fixed.

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

The uid parameter when extracted, has line separators which does not allow for a proper check with the one stored locally, we are merely removing those from the received value.

String changes made/needed

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9039863 [details]
Bug 1519418 - Replace line separators from the uid parameter received from the deeplink intent. r=sdaswani

OK for uplift to beta 8; part of fix for bug 1515651.

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f853d6211595

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/71f45dc5eca0

Comment 14

[Levente Sacal]

Tested on the latest 66 Beta 11 and latest Nightly 67.0a1 (2019-02-27). I have received the push notifications but the notification does not open any link.

Comment 15

[Vlad Baicu]

Attachment: Bug 1519418 - Replace MMA preferences with sharedPreferences.r=JanH

Patch D16319 attempted to clean the MMA code and allow for easier debugging
of LP deeplinks on dev builds. However it introduced a regression because the
MMA preferences were being accessed by the initializing activity. By making
getDeviceId public and static, calling it from another activity would result
in a null value returned. I have refactored the code to use shared preferences
and remove the dependency on other activities.

Comment 16

[Vlad Baicu]

Comment on attachment 9048261 [details]
Bug 1519418 - Replace MMA preferences with sharedPreferences.r=JanH

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1519418 first patch
• User impact if declined: Patch from bug 1515651 will not work.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Send a deeplink from the campaign set up in bug 1515651.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are reverting to the previous approach used.
• String changes made/needed:

Comment 17

[Vlad Baicu]

I have submitted an uplift request for beta in order to have this align with the next release mid March as this patch is needed in order to achieve a succesful landing of the patch from bug 1515651.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks Vlad, looks like this should also land on mozilla-central so if you can check it in there, we can uplift after it's in nightly.

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

[Tracking Requested - why for this release]:

Comment 20

[Vlad Baicu]

The feature added in bug 1515651 is expected by the marketing dept to land on 66. I have submitted the uplift request before landing on nightly so that after it lands, we can confirm it works and uplift afterwards, thus sparing some time.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

ok, what we mean by "uplift" is backporting code to beta or release branches which is what we're doing here.
I don't understand - why not also land it on mozilla-central?

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9048261 [details]
Bug 1519418 - Replace MMA preferences with sharedPreferences.r=JanH

I'm fine with landing this on beta, though we also need it on m-c.

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

I think I get what you intended now. You were just putting in the uplift request and this is likely going to land on m-c tomorrow.

Comment 24

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9a68634dc411

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Still not seeing this land on m-c - can you land it?

Comment 26

[Vlad Baicu]

Seems like the checkin-needed got cleared when it landed on beta, I've reapplied it.

Comment 27

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/3c21d47f9ddf9853519f2f7c49a1fe1ca5aa6463

Comment 28

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3c21d47f9ddf