Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1810047 - Implement UI for webextensions optional permissions. #3917

Merged
merged 2 commits into from Oct 4, 2023

Conversation

willdurand
Copy link
Member

@willdurand willdurand commented Oct 3, 2023

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not

After merge

  • Breaking Changes: If this is a breaking Android Components change, please push a draft PR on Reference Browser to address the breaking issues.

To download an APK when reviewing a PR (after all CI tasks finished running):

  1. Click on Checks at the top of the PR page.
  2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
  3. Click on the build-apk-{fenix,focus,klar}-debug task you're interested in.
  4. Click on View task in Taskcluster in the new DETAILS section.
  5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

https://bugzilla.mozilla.org/show_bug.cgi?id=1810047

@willdurand
Copy link
Member Author

image

@github-actions github-actions bot added the work in progress Not ready to land yet. Work in progress (WIP). label Oct 3, 2023
@willdurand willdurand force-pushed the optional-prompt branch 2 times, most recently from 4e653d2 to 8b6440a Compare October 3, 2023 21:03
@willdurand willdurand marked this pull request as ready for review October 4, 2023 12:56
@github-actions github-actions bot added 🕵️‍♀️ needs review PRs that need to be reviewed and removed work in progress Not ready to land yet. Work in progress (WIP). labels Oct 4, 2023
@Amejia481 Amejia481 self-requested a review October 4, 2023 17:34
Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

As chatted via Slack, we will further investigate if we should auto grant directly on gecko/geckoview. At the moment, we are duplicating this functionality in Desktop and Fenix, eventually we will get Gecko on iOS. This seams to be a core functionality that shouldn't left to handle to consumers.

@Amejia481 Amejia481 added approved PR that has been approved and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Oct 4, 2023
@willdurand willdurand added the 🛬 needs landing PRs that are ready to land label Oct 4, 2023
@mergify mergify bot merged commit 23d8621 into mozilla-mobile:main Oct 4, 2023
108 of 109 checks passed
@willdurand willdurand deleted the optional-prompt branch October 4, 2023 18:42
@pmarks-net
Copy link
Contributor

Thanks for fixing my extension, but be aware that the permissions granted via this dialog are not visible (and thus cannot be revoked) from the extension's Permissions UI.

@willdurand
Copy link
Member Author

and thus cannot be revoked

the extension can revoke them

@willdurand
Copy link
Member Author

@pmarks-net sorry for the quick reply the other day, to expand on that: yes, you're right. We don't have UI for optional permissions in the Fenix details view. There is a bug for that, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1811563. We'll get that fixed eventually but I don't have an ETA yet. In the meantime, extensions can offer optional permission removal using permissions.remove().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved PR that has been approved 🛬 needs landing PRs that are ready to land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants