1511165 - about:welcome addon card on windows says "addon name"

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P2)

Description

[Shane Caraveo (:mixedpuppy)]

Attachment: Screen Shot 2018-11-29 at 3.58.52 PM.png

see attached image.  The raw data I get from the attribution service does have the proper name.  Clicking the download button downloads the correct extension.

Comment 1

[Ursula Sarracini (:ursula)]

Maybe because of bug 1511156 ?

Comment 2

[Shane Caraveo (:mixedpuppy)]

(In reply to Ursula Sarracini (:ursula) from comment #1)
> Maybe because of bug 1511156 ?

I thought that at first, but the download button works fine.

Bug 1511156 affects the extension id in the attribution data, which we use in an API call to AMO to retrieve the rest of the data, which includes the extension name, download link, etc.  The data retrieved via the AMO API is not encoded.

about:welcome should also be a little more resilient.  It currently fails so show anything if it cannot get the addon data.  I'm not clear if that is intentional.  I guess from a user perspective its fine, but it threw me through a loop while trying to figure out what was happening.

Comment 3

[Shane Caraveo (:mixedpuppy)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #2)

> I thought that at first, but the download button works fine.

I mean the "add extension" button as shown in the screenshot.

Comment 4

[Ursula Sarracini (:ursula)]

Maybe it's a problem with l10n... if getting the addon name fails it should return null, not "addon-name" so that's bizarre. I'll take a look.

Comment 5

[emanuela [ux]]

Can we run some tests to identify how often this scenario occurs? It's important to understand if it's just an edge case or something more common. 

Anyhow, when we are not able to load the extension name, I suggest using something more neutral like 'Now let's get your extension.'
NI Meridel on the bug to confirm the new string.

Comment 6

[Andrei Oprea [:andreio]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #2)
> 
> > I thought that at first, but the download button works fine.
> 
> I mean the "add extension" button as shown in the screenshot.

Addon name [0] and addon install url [1] are fetched independently of each other so it's not unlikely that one of them fails, in this case the name.

[0] https://github.com/mozilla/activity-stream/blob/608aef966d0cf4f59dda623c50602530dc2cc623/lib/OnboardingMessageProvider.jsm#L16
[1] https://github.com/mozilla/activity-stream/blob/608aef966d0cf4f59dda623c50602530dc2cc623/lib/ASRouter.jsm#L809

Comment 7

[Shane Caraveo (:mixedpuppy)]

Can you fix that so that a) we do not make multiple http calls to get data for a single extension, and b) we use the API rather than fetch?  Are there any other API calls for other addon data items?

Comment 8

[Shane Caraveo (:mixedpuppy)]

(In reply to emanuela [ux] [OOO 12/21 - 01/07] from comment #5)

> Anyhow, when we are not able to load the extension name

By my tests the extension name is in the data returned so this would be a bug somewhere in the activity stream code, not a lack of having access to the name.

Comment 9

[Meridel [:meridel]]

If we cannot obtain the extension name for whatever reason, the string should be:

Now let's get your extension.


If we cannot obtain the theme name for whatever reason, the string should be:

Now let's get your theme.

(if it is possible to differentiate?)

Comment 10

[Shane Caraveo (:mixedpuppy)]

I'm sorry to disagree.  Given bug 1511173, I think that if we do not have all the data from the AMO api to indicate to the user what they are installing, we should not show the addon card at all.  Even without that bug, I feel like having an install button where the user has no idea what is being installed is not good.

Comment 11

[Meridel [:meridel]]

Aha, I see your point, and I concede. No add-ons card shown if we can't retrieve the AMO data.

Comment 12

[Amy Tsay [:amyt]]

Sounds good. If we were landing people back on an AMO product page the generic copy might be ok. But if the install is taking place right from about:welcome, better not to show if we can't fetch the data.

Comment 13

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4589

Comment 14

[emanuela [ux]]

Thank you for adding extra context, Shane. I agree with you and Meridel.

Comment 15

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/68cef93e3fba3dce03abfd2dd646ee6074a1693f
Fix Bug 1511165 - about:welcome addon card on windows says "addon name" (#4589)

Comment 16

[Vlad Jiman]

We will verify this bug as soon as it's possible to do so on Windows.

Comment 17

[Ed Lee :Mardak]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Is this something we need to uplift to Beta?

Comment 19

[Ed Lee :Mardak]

I would think not needed for beta 65 given that it'll be turned off for 65 with bug 1515411… yeah?

Comment 20

[Andrei Oprea [:andreio]]

Yes, no uplift needed.

Comment 21

[Vlad Jiman]

Verified using Nightly 66.0a1 (build 20190120213632) on Windows 10 x64 using multiple extensions, the issue is no longer reproducible. Attaching screenshot

Comment 22

[Vlad Jiman]

Attachment: validated.png