1551563 - "Manage shortcuts" at about:addons shows hidden extensions/built-in search engines

Status: VERIFIED FIXED

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

Description

[Rob Wu [:robwu]]

STR:

1. Visit about:addons.
2. Click on the Extensions tab.
3. Click on the gears and then click on the "Manage Extension Shortcuts" menu item.

Expected:

• Should only show items that also appear in the extension list.
• Should not show hidden extensions, except if they support keyboard shortcuts.

Actual:

• Shows hidden extensions, even if they do not support shortcuts:

The following extensions do not have shortcuts:

• Amazon.com
• Bing
• DuckDuckGo
• eBay
• Google
• hotfix-update-xpi-intermediate
• Twitter
• Wikipedia (en)

The hidden/built-in search engines are from bug 1486820.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Not a big deal, but should be addressed in the html version. Mark, can you add this in to your list?

Comment 2

[Rob Wu [:robwu]]

I'll take this, to prevent the lot of built-in search extensions from appearing in 68.

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1551563 - Support useAddonManager:"builtin"

In the bug 1551563, I needed the ability to load a hidden extension in a
test. Despite setting the "isPrivileged" flag, the "hidden" flag was
ignored, because "useAddonManager" was set, which results in the
installation of unsigned add-ons.

To be able to test hidden/privileged add-ons, this patch introduces
support for the useAddonManager: "builtin" to the loadExtension
method of ExtensionTestUtils. This allows browser tests and xpcshell
tests to simulate privileged extensions.

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1551563 - Move shortcut test to own file

In bug 1522757, a new test was added to verify that extensions without
shortcuts were hidden in the shortcut view, and shown in a list of
extensions without shortcuts.

The test was inserted in an existing test that did not have anything to
do with the change. In the next patch, I am going to add similar tests,
so the test logic was moved to a separate file.

Comment 5

[Rob Wu [:robwu]]

Attachment: Bug 1551563 - Hide hidden/builtin extensions in shortcuts view

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e43b0f4322b
Move shortcut test to own file r=mstriemer,mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/9a811a4d134d
Hide hidden/builtin extensions in shortcuts view r=mstriemer,mixedpuppy,kmag

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/6e43b0f4322b
https://hg.mozilla.org/mozilla-central/rev/9a811a4d134d

Comment 8

[Rob Wu [:robwu]]

Comment on attachment 9066053 [details]
Bug 1551563 - Hide hidden/builtin extensions in shortcuts view

Beta/Release Uplift Approval Request

• User impact if declined: The "Manage shortcuts" view contains a list of built-in search engines, starting from Firefox 68 because of bug 1486820 .
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: comment 0 has STR if QE wants to verify.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): 1 line change in UI code, plus tests.
• String changes made/needed:

Comment 9

[Julien Cristau [:jcristau]]

Does this address both the old and new about:addons?

Comment 10

[Rob Wu [:robwu]]

(In reply to Julien Cristau [:jcristau] from comment #9)

Does this address both the old and new about:addons?

Yes it does. The "Manage shortcuts" view is written in HTML already (not XUL), and used by both the old and new about:addons.

Comment 11

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 68.0a1 (2019-05-14) on Windows 7, 64 Bit. The fix of this bus is verified with latest Nightly!

Build ID : 20190602221035
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9066053 [details]
Bug 1551563 - Hide hidden/builtin extensions in shortcuts view

addons manager fix for 68.0b7

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fe605c689819
https://hg.mozilla.org/releases/mozilla-beta/rev/5532b4c4315e

Comment 14

[Alex Cornestean]

Verified the fix on the latest versions of Nightly (69.0a1 / 20190604214415) and Beta (68.0b7 / 20190603181408) under Windows 10 Pro 64-bit and macOS High Sierra 10.13.6.

The fix appears to be valid as when accessing the "Manage Extension Shortcuts" page from the gears menu, all installed extensions which support shortcuts are properly displayed and all installed extensions which do not support shortcuts are listed below, under the ‘The following extensions do not have shortcuts:‘ section. Any uninstalled extensions lacking shortcut support will no longer be displayed under the above mentioned section as in previous builds.

However, there is a slight difference between the Nightly and the Beta versions, as on Nightly, a ‘Firefox Screenshots’ extension with shortcut support is present among the installed extensions which support shortcuts. Please see the attached screenshots for further details.

Can you please confirm this is the correct behavior before validating the fix and marking the issue as Verified Fixed?

Comment 15

[Alex Cornestean]

Attachment: Nightly - FF Screenshots - present.png

Comment 16

[Alex Cornestean]

Attachment: Beta - FF Screenshots - missing.png

Comment 17

[Rob Wu [:robwu]]

(In reply to Alex Cornestean from comment #14)

However, there is a slight difference between the Nightly and the Beta versions, as on Nightly, a ‘Firefox Screenshots’ extension with shortcut support is present among the installed extensions which support shortcuts. Please see the attached screenshots for further details.

Can you please confirm this is the correct behavior before validating the fix and marking the issue as Verified Fixed?

This is as expected. This bug has a "see also" to bug 1553416, which adds Screenshots to the "Manage shortcuts" view.
Although the patch for that bug is simple, I did not request an uplift because it was not a regression (unlike this bug).