[about:addons] Show recommendations on the list view

VERIFIED FIXED in Firefox 68

Status

()

enhancement
P1
normal
VERIFIED FIXED
2 months ago
Last month

People

(Reporter: mstriemer, Assigned: mstriemer)

Tracking

(Blocks 2 bugs)

Trunk
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 verified, firefox69 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Recommendations should also be included in the list view for extensions and themes. This can re-use a lot of code from the recent discovery pane in HTML updates.

Blocks: 1555012
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/502619bc75a8
Show recommendations on extension and theme lists r=robwu,flod,jaws
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Posted image Bug1550911.gif

This issue is verified as fixed on Firefox 69.0a1 (20190604214415) under Win 7 64-bit and Mac OS X 10.14.1.

The preference “extensions.htmlaboutaddons.recommendations.enabled” is set to true by default.

When the preference is set to true, the recommendations for extensions and themes are displayed in the “Extensions” and “Themes” tabs.

Please see the attached video.

Status: RESOLVED → VERIFIED
Blocks: 1557369

Comment on attachment 9064206 [details]
Bug 1550911 - Show recommendations on extension and theme lists r?robwu

Beta/Release Uplift Approval Request

  • User impact if declined: Users will see an empty page when no extensions are installed on the extension list in HTML about:addons, there won't be recommendations on the extension or theme list.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: bug 1557369
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Has test coverage, mostly involves showing parts of existing code on another page.
  • String changes made/needed: Yes
Attachment #9064206 - Flags: approval-mozilla-beta?

Bug 1557369 should also be uplifted on top of this, with some test-only changes.

flod: this has new strings, so I'm not sure it's still ok to uplift this late...

Flags: needinfo?(francesco.lodolo)

At this point I don't think it's a localization call, it's for product and RelMan to decide if they are OK with the risk. FYI, we're 2 weeks from the deadline to update any localization for 68.

Flags: needinfo?(francesco.lodolo)

I'd like this patch (from comment 1) to be uplifted (even if we end up turning the feature off by default), because the patch contains changes that are depended on by other patches/tests.

I've added a patch in comment 9 in case the string changes result in a rejected uplift.

Locally I verified that all relevant tests (i.e. those in toolkit/mozapps/extensions/test/browser) still pass (after applying the patches from this bug on the Beta branch), and that the header before the recommendations now shows "Recommendations" instead of Recommended Extensions/Themes.

(In reply to Rob Wu [:robwu] from comment #9)

  • Reuse generic "Recommendations" string from the tab on the sidebar
    instead of "Recommended Extensions" / "Recommended Themes".

Sorry, that's not going to work. You're taking a string from a different context, and with space constraints on top of it.

I didn't check other language, but this would make no sense in Italian (there's a noun in the sidebar, not an adjective).

I've updated the patch to use another close equivalent from ActivityStream. The contexts are comparable; they are both used as the heading of a section of recommended items (ref: AS).

  • Current: Recommended Extensions / Recommended Themes
  • Proposed: Recommended by Firefox (or Recommended by Nightly).
    (this includes the product name, which we already do on Android: "Browse Firefox’s Recommended Extensions".)

An alternative string is Recommended Extension, used in CFR. The only issue with this is that it is singular, not plural.

Please stop trying to reuse strings out of context, that's not a solution. A title might use a different case than a tooltip, and a different form (descriptive vs imperative, noun vs verb). Adjectives need to match the noun, and there are two here (extension, them), with potentially different genders.

As said, this is a product+relman decision. If you need this in 68, you should uplift the patch with strings, and deal with the risk associated to it.

Attachment #9071114 - Attachment is obsolete: true

I'd like the strings to be translated to at least French and German before taking this in beta.

The strings are localized to French and German. Product wants this, and Julien has set the bar in #c14.

Comment on attachment 9064206 [details]
Bug 1550911 - Show recommendations on extension and theme lists r?robwu

approved for 68.0b10

Attachment #9064206 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is verified as fixed on Firefox 68.0b10 (20190613141208) under Win 7 64-bit and Mac OS X 10.14.1.

You need to log in before you can comment on or make changes to this bug.