1911163 - Show to the user the full list of domains the extension will have access to in the install dialog

Status: VERIFIED FIXED

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

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

Currently the install dialog only shows a subset of the domains that the extension being install will be granted access to once fully installed.

The goal of this issue is to give to the user more complete visibility of all the websites that the extension they are installing will have access to, while the addon is not installed yet, by including in the add-on install dialog the entire list.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1911163 - Refactor addon permissions dialog DOM elements manipulations out of ExtensionsUI.sys.mjs. r?willdurand!

Depends on D218367

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1911163 - Split addon-webext-permissions-notification custom element render method. r?willdurand!

Depends on D218591

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1911163 - Add full domains list in addon-webext-permissions-notification permissions list.

Depends on D218592

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: WIP: Bug 1911163 - moz-scroll-shadows-box reusable widget.

This patch includes a prototype of a reusable widgets providing the
kind of scroll shadows part of the current figma designs
(See https://www.figma.com/design/2FeZiu4V1CAqZ0zzVcH7sW/Manifest-V3?node-id=3399-10154&t=GyTm7596Ot84dkX7-0
and https://www.figma.com/design/2FeZiu4V1CAqZ0zzVcH7sW/Manifest-V3?node-id=3159-8355&t=GyTm7596Ot84dkX7-0)

Besides a slightly different kind of scroll shadow part of the "Shopping Card" domain
specific widget, see storybook story here:
https://firefoxux.github.io/firefox-desktop-components/?path=/story/domain-specific-ui-widgets-shopping-shopping-card--card-type-show-more

This kind of pattern doesn't seem to be used anywhere else, and so I'm a bit conflicted about introducing
it just for the full domains list permissions meant to be added to addon install dialog as part of this
bugzilla issue, and so I decided to prototype it in a separate patch and as a reusable widget (instead of
rolling it into the dialog content custom element definition) and to include in this patch a new small
set of storybook stories to allow others to more easily try out the "moz-scroll-shadows-box" wrapper element
behaviors on a scrollable and non scrollable slotted element in isolation (while a separate patch includes
additional changes to show its usage in the addon install dialog domains list).

Depends on D218593

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: WIP: Bug 1911163 - Add use of the moz-scroll-shadows-box widget for the domains list included in the addon install dialog.

Depends on D218856

Comment 6

[Phabricator Automation]

Comment on attachment 9417818 [details]
Bug 1911163 - Refactor addon permissions dialog DOM elements manipulations out of ExtensionsUI.sys.mjs. r?willdurand!

Revision D218591 was moved to bug 1915662. Setting attachment 9417818 [details] to obsolete.

Comment 7

[Phabricator Automation]

Comment on attachment 9417819 [details]
Bug 1911163 - Split addon-webext-permissions-notification custom element render method. r?willdurand!

Revision D218592 was moved to bug 1915662. Setting attachment 9417819 [details] to obsolete.

Comment 8

[Phabricator Automation]

Comment on attachment 9418364 [details]
WIP: Bug 1911163 - Add use of the moz-scroll-shadows-box widget for the domains list included in the addon install dialog.

Revision D218857 was moved to bug 1916691. Setting attachment 9418364 [details] to obsolete.

Comment 9

[Phabricator Automation]

Comment on attachment 9418363 [details]
WIP: Bug 1911163 - moz-scroll-shadows-box reusable widget.

Revision D218856 was moved to bug 1916691. Setting attachment 9418363 [details] to obsolete.

Comment 10

[Phabricator Automation]

Comment on attachment 9418364 [details]
WIP: Bug 1911163 - Add use of the moz-scroll-shadows-box widget for the domains list included in the addon install dialog.

Revision D218857 was moved to bug 1916691. Setting attachment 9418364 [details] to obsolete.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a31d48c96789
Add full domains list in addon-webext-permissions-notification permissions list. r=desktop-theme-reviewers,fluent-reviewers,willdurand,bolsson,sfoster

Comment 12

[Atila Butkovits]

Backed out for causing failures at browser_parsable_css.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/f74bbe0aed7ff26478a235833d1a551cca6d389c

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=a31d48c96789e3d63726cd05631882a950bd672b&selectedTaskRun=cAf04wNMTQytGt0MldhJJg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=476422671&repo=autoland&lineNumber=2269

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

I looked into the browser_parsable_css.js and as I was guessing it was hit due to a max-height: auto in the new CSS rules (because auto isn't actually a valid value for max-height).

The max-height: auto is not even needed to be explicitly part of the CSS rule and so I have removed in an updated version of the phabricator revision.

We will reland the revision later today.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcfd93872dad
Add full domains list in addon-webext-permissions-notification permissions list. r=desktop-theme-reviewers,fluent-reviewers,willdurand,bolsson,sfoster

Comment 15

[Sandor Molnar[:smolnar]]

Backed out for causing perma bc failures @ browser_permissions_dismiss.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/651540cbb81a3f8823cd8df3a46533d8feca312b

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | browser/base/content/test/webextensions/browser_permissions_dismiss.js | Permissions list should have 6 entries - Got 5, expected 6

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Sandor Molnar[:smolnar] from comment #15)

Backed out for causing perma bc failures @ browser_permissions_dismiss.js

I took a look and confirmed that a few tests in browser/base/content/test/webextensions (in addition to browser_permissions_dismiss.js mentioned in comment 15) needs to be adjusted to account for the way host permissions are expected to be listed in the install permissions
dialogs with the changes introduced as part of this bugzilla issue (similarly to other tests that were already adjusted in the current version of the patch).

There are a couple of tests from that test dir that needs a bit more work to be adjusted, and so I'll be come back to this tomorrow and prepare a new update of the patch attached to this bug before pushing it again to autoland.

As an additional side note: this backout also prompted us to double-check if browser/base/content/test/webextensions was missing from our webextensions try preset, and that seems to be definitely the case (https://searchfox.org/mozilla-central/rev/3966e5534ddf922b186af4777051d579fd052bad/tools/tryselect/try_presets.yml#326-335) and so I'll also file a separate bug and update the preset to make it harder to miss tests from browser/base/content/test/webextensions in our try pushes (which are often enough created using the webextensions try preset).

Comment 17

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1911163 - Adapt tests from browser/base/content/test/webextensions/ to pass on old and new install dialog designs. r?willdurand!

Depends on D218593

Comment 18

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1911163 - Add browser/base/content/test/webextensions/ to the webextensions try preset. r?willdurand!

Depends on D224465

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cb8a6bdfa50
Add full domains list in addon-webext-permissions-notification permissions list. r=desktop-theme-reviewers,fluent-reviewers,willdurand,bolsson,sfoster
https://hg.mozilla.org/integration/autoland/rev/e218529b850c
Adapt tests from browser/base/content/test/webextensions/ to pass on old and new install dialog designs. r=willdurand
https://hg.mozilla.org/integration/autoland/rev/919650078b37
Add browser/base/content/test/webextensions/ to the webextensions try preset. r=willdurand

Comment 20

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/7cb8a6bdfa50
https://hg.mozilla.org/mozilla-central/rev/e218529b850c
https://hg.mozilla.org/mozilla-central/rev/919650078b37

Comment 21

[Alex Cornestean]

Verified as Fixed. Tested on the latest Nightly (133.0a1/20241020211110) under Windows 10, Ubuntu 22.04 LTS and macOS 11.3.1.

Several scenarios have been tested as part of this feature for both the install dialog and the optional permissions request dialog, with the following results:

• when the add-on does not require any host permissions -> no host permissions are shown in the dialogs
• when the “all-urls” permission is required -> the “Access your data for all websites” string will be displayed in the dialogs
• when only one host permission is required -> the “Access your data for sites in X domains” will be displayed in the dialogs
• when more than 1 but less than 6 host permissions are required -> a bordered non-scrollable list is displayed in the dialogs, showing the entirety of domains the add-on will have access to
• when 6 or more host permissions are required -> a bordered scrollable list is displayed in the dialogs, allowing the user to scroll to reveal the entirety of domains the add-on will have access to
• when 6 or more host permissions are required and the “all-urls” permission is also requested -> the “Access your data for all websites” string will be displayed in the dialogs as the “all-urls” permission will include all domains