1560746 - "aAddon.name is undefined" break the new Extensions pane when specific legacy add-ons is installed

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[YF (Yang)]

STR:
See bug 1517988, same reason.

Actual results:
In recent Nightly, the Add-ons Manager - Extensions pane is broken.

Expected results:
Be able to display and operate correctly. If possible, extensive compatibility with this special case, and make a testcase.

Comment 2

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

[Tracking Requested - why for this release]: Add-on management becomes inaccessible.

Comment 3

[Rob Wu [:robwu]]

Could you provide the exact steps to reproduce the problem? I tried but cannot.

I did the following:

1. Create directory on Linux, and start Firefox 56.0.2 with it.
2. Install https://addons.mozilla.org/en-US/firefox/addon/disabled-add-on-fix-52-56/
3. Install a signed legacy add-on that I downloaded from the dev hub (version 0.1 of No Native Notifications).
4. Close Firefox.
5. Start Firefox Nightly 69.0a1 buildID 20190624213657
6. Visit about:addons and click on Extensions.

The two legacy add-ons show up as expected (and they are of course disabled).

Comment 4

[Julien Cristau [:jcristau]]

I guess the issue is here:

  sortByFn(aAddon, bAddon) {
    return aAddon.name.localeCompare(bAddon.name);
  }

where some addons have a null name, as in bug 1517988? It doesn't look like I can download version 0.1 of no native notifications, so not sure if it qualifies, but it sounds from the reporter as if attachment 9034590 [details] would reproduce?

Comment 5

[Rob Wu [:robwu]]

I cannot reproduce with the add-on from comment 4.
I can manually reproduce if I install 2 addons, and then edit extensions.json in the profile, and remove the "name" key under the "defaultLocale" keys of one of the add-ons.

YF, could you attach extensions.json from the affected profile?

Comment 6

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

As far as I can see, this needs a localized (non en-US) Firefox. Ping me if you want either a profile which reproduces the issue or access to a test Sync account with which the issue can be reproduced.

Comment 7

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

Attachment: extensions.json

From the extensions.json:

    {
      'id': '[e-mail]',
...
      'defaultLocale': {
        'name': 'Extension Test',
      },
...
      'locales': [
        {
          'locales': [
            'en-US'
          ],
          'name': 'Extension Test',
...
        {
          'locales': [
            'de'
          ],
          'name': null,
...

Comment 8

[Rob Wu [:robwu]]

Thanks for the info. This appears to be a regression by bug 1514316 (behind a pref), and enabled by default by bug 1555012. It affects profiles with legacy add-ons.

To reproduce:

1. Create directory on Linux, and start Firefox 56.0.2 with it.
2. Install https://addons.mozilla.org/en-US/firefox/addon/disabled-add-on-fix-52-56/
3. Install https://bugzilla.mozilla.org/attachment.cgi?id=9074926 (=addon with ID [e-mail] version 2.16.1-signed.1-let-fixed.1-signed).
4. Close Firefox.
5. Start Firefox Beta (I tested with 68.0b13) -- Nightly does not support automatic langpack downloads. If you want to use Nightly, see step 3 of 1524679#c2.
6. Visit about:preferences and change the locale to Chinese (China) (zh-cn), German (de), French (fr) or Dutch (nl).
7. Restart Firefox.
8. If the extensions list at about:addons still looks fine, install any other add-on, so that the extension-test add-on is not the last in the list.
9. Disable the just-installed add-on (now you should have 3 disabled add-ons: from step 2, step 3 and step 8).
10. Refresh the about:addons page.

Expected:

• The extension list should be shown

Actual:

• The extension list is blank.
• The global JS console shows TypeError: aAddon.name is null aboutaddons.js:2117:5

More info:
I looked up the XPI for the add-on and found that the specific add-on is a legacy add-on with the metadata in install.rdf, where the localized value may be unset:

<Description ... em:id="[e-mail]" em:name="Extension Test" ...>
        <em:localized>
          <Description em:locale="en-US" em:name="Extension Test"  ... />
        </em:localized>
        <em:localized>
          <Description em:locale="zh-CN" ... /> (note: no em:name)
        </em:localized>

The name for a WebExtension is always a string, so this bug can only be reproduced with profiles containing legacy add-ons.

Comment 9

[Rob Wu [:robwu]]

Attachment: Bug 1560746 - Fall back to default locale if localized name is missing

Legacy add-ons can have null values instead of localized strings.
This breaks code that expects names to always be a string.
Fix this by falling back to the default locale.

This code is not needed for WebExtensions, because the name for those is
always a string, even if the translation is missing.

Comment 10

[Rob Wu [:robwu]]

Patch is up and got an amazingly fast review.

Try job: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebfc4e13b4b0019ab81f0f1f1586c4d2f95076d1

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7598ffdf3ca1
Fall back to default locale if localized name is missing r=kmag

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/7598ffdf3ca1

Comment 13

[Rob Wu [:robwu]]

Comment on attachment 9074651 [details]
Bug 1560746 - Fall back to default locale if localized name is missing

Beta/Release Uplift Approval Request

• User impact if declined: Some users are not able to use about:addons to manage extensions. Specifically, this affects users whose profile contain partially localized legacy add-ons.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment https://bugzilla.mozilla.org/show_bug.cgi?id=1560746#c8
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple change, tests are passing, manually verified by developer before landing.
• String changes made/needed: none

Comment 14

[Rob Wu [:robwu]]

Attachment: extension_test-2.16-tb+fn+fx+sm.xpi

Add-on for use with STR in comment 8.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9074651 [details]
Bug 1560746 - Fall back to default locale if localized name is missing

fix an issue with the new about:addons, approved for 68 rc1

Comment 16

[Alex Cornestean]

Verified the fix on the latest versions of Nightly (69.0a1 / 20190630214222) and Beta (68.0b14 / 20190627143605) under Windows 10 Pro 64-bit and Ubuntu16.04.6 LTS, following the provided STR, with Chinese, German, French and Dutch locales.

The issue is verified as FIXED on the latest Nightly version where, after proceeding with the reproduction method and refreshing the Extensions Tab, the disabled add-ons list is still properly displayed.

The issue is consistently reproducible on the latest Beta version, obtaining the exact results specified in Comment 8.

Comment 17

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

https://hg.mozilla.org/releases/mozilla-release/rev/5b0b86471dae

Comment 18

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

https://hg.mozilla.org/releases/mozilla-esr68/rev/5b0b86471dae

Comment 19

[Alex Cornestean]

Verified the fix on the latest Beta Release Candidate (68.0 / 20190701181138) and ESR (68.0esr / 20190704173630) under Windows 10 Pro 64-bit and Ubuntu 16.04.6 LTS, following the provided STR, with Chinese, German, French and Dutch locales.

The issue is verified as FIXED on the above mentioned versions as the disabled add-ons list is still properly displayed upon proceeding with the STR provided in Comment 8.