Fenix: [Bug] Remember permissions for sites that requests multiple permissions at the same time does not work

[rfugger]

Steps to reproduce

Go to site requesting location (eg, castanet.net). Select "do not share" and "remember for this site".

Expected behavior

Should never be asked again at that site.

Actual behavior

I get asked again each day. I do not get asked again if I visit immediately. But if I wait a day, I am asked for my location again.

Device information

• Android device: Google Pixel fully updated
• Fenix version: 79.0.5

[mcarare]

Eng note: In the case of this website it appears that sometimes the wrong permission is saved on clicking prompt for location permissions. contentPermissionRequest is for youtube and PERMISSION_AUTOPLAY_INAUDIBLE. @Amejia481 do you have some insight on how that might happen, that can help me investigate further? Thank you!

[Amejia481]

Eng note: In the case of this website it appears that sometimes the wrong permission is saved on clicking prompt for location permissions. contentPermissionRequest is for youtube and PERMISSION_AUTOPLAY_INAUDIBLE. @Amejia481 do you have some insight on how that might happen, that can help me investigate further? Thank you!

What is happening is that we have two requests and they are overridden each other:

1. We get a location request from castanet.net and we store it on Session.contentPermissionRequest.
2. We show the permission dialog for location permission (The location request is still pending).
3. We get another permission request from youtube.com/embed/ that it is an iframe on castanet.net that overrides the location permission on the Session.contentPermissionRequest.

Unfortunately, in they way that permissions request were designed wasn't accounting that a site could request multiple permissions at the same time, this is a special case as it is not the site itself that is requesting the permission it's an embed one on iframe

<iframe id="ytframe" src="https://www.youtube.com/embed/A7XIZ8YAzLA?wmode=transparent&amp;rel=0&amp;showinfo=0&amp;color=white&amp;enablejsapi=1" allowfullscreen="" width="320" height="180" frameborder="0"></iframe>

To address this issue we will need to improve how we are handling and consuming permission requests to allow this use case, be aware that we are migrating from Session to SessionState , maybe this could be an opportunity to migrate feature-sitepermissions to use the browser-state as we are not adding more new functionality to Session, as it makes the inevitable migration longer. Here is an example where we migrated feature-downloads to use browser-state

[Amejia481]

If there is anything in what I can help just let me know happy to help :)

[codrut-topliceanu]

Ok, I've been working on this for the past week trying to migrate SitePermissionsFeature to sessionState using Kotlin flow
(Engine observer -> UpdatePermissionRequests(newRequest) to ContentStateReducer updating ContentState with new permissionRequest and collecting it in SitePermissionsFeature).

However session is too tightly coupled with SitePermissionsFeature, every method uses session in one way or another, I have managed to replace it in most places but I'm not very happy with the results. I may have to do a re-write of the feature in order to move away completely from session.

Another issue is
private fun PermissionRequest.getHost(session: Session) = (uri?.toUri()?.host ?: session.url.toUri().host ?: "")
because in the case of embedded elements the requested permission's uri and session.url are not the same.

---

1. We get a location request from castanet.net and we store it on Session.contentPermissionRequest.
2. We show the permission dialog for location permission (The location request is still pending).
3. We get another permission request from youtube.com/embed/ that it is an iframe on castanet.net that > overrides the location permission on the Session.contentPermissionRequest.

While the switch to Kotlin Flow has fixed this issue, there is something else that I would require further input on, and that is how we store permission decisions.

If a user navigates to a restaurant's webpage, and there is an embedded google maps requesting a location permission to which the user replies with _Deny + Remember decision for this site_ how should that decision be remembered?:

• only for the restaurant's webpage -> any location request made while the user is on the restaurant's webpage will be automatically blocked, even if they are made by the webpage or another embedded element
• for the google maps embedded -> all google map embedded location requests on ANY site will be automatically blocked
• or for a combination of both? -> all google map embedded location requests on the restaurant's webpage will be blocked

[Amejia481]

Ok, I've been working on this for the past week trying to migrate SitePermissionsFeature to sessionState using Kotlin flow
(Engine observer -> UpdatePermissionRequests(newRequest) to ContentStateReducer updating ContentState with new permissionRequest and collecting it in SitePermissionsFeature).

However session is too tightly coupled with SitePermissionsFeature, every method uses session in one way or another, I have managed to replace it in most places but I'm not very happy with the results. I may have to do a re-write of the feature in order to move away completely from session.

Another issue is
private fun PermissionRequest.getHost(session: Session) = (uri?.toUri()?.host ?: session.url.toUri().host ?: "")
because in the case of embedded elements the requested permission's uri and session.url are not the same.

No problem 👍 , feel free to do the adaptations needed. If it helps you could open a draft pr and we could help you by giving you early feedback.

While the switch to Kotlin Flow has fixed this issue, there is something else that I would require further input on, and that is how we store permission decisions.

If a user navigates to a restaurant's webpage, and there is an embedded google maps requesting a location permission to which the user replies with Deny + Remember decision for this site how should that decision be remembered?:

only for the restaurant's webpage -> any location request made while the user is on the restaurant's webpage will be automatically blocked, even if they are made by the webpage or another embedded element
for the google maps embedded -> all google map embedded location requests on ANY site will be automatically blocked
or for a combination of both? -> all google map embedded location requests on the restaurant's webpage will be blocked

I think before making a decision we could take a look to what Fennec/Desktop and other browsers do in this situation, and try to see which approach fits better for us.

[codrut-topliceanu]

@Amejia481 Sorry for the delay, I managed to put together a passable PR for this issue (https://github.com/mozilla-mobile/android-components/pull/8555). I still have to rebase and correct some tests, but if you're willing to take a quick look at it I'd appreciate it.

[Amejia481]

@Amejia481 Sorry for the delay, I managed to put together a passable PR for this issue (mozilla-mobile/android-components#8555). I still have to rebase and correct some tests, but if you're willing to take a quick look at it I'd appreciate it.

Thanks for putting the pr, I will take a look :)

[codrut-topliceanu]

Reopened for testing.
Related AC issue https://github.com/mozilla-mobile/android-components/issues/8554

[codrut-topliceanu]

Fix will be available with AC 65

[AndiAJ]

In this case, I'll remove the QA needed label until further notice.

[codrut-topliceanu]

It seems AC 65 is now in nightly, so I'll add the tag again.

[sflorean]

Verified as fixed on Nightly 11/5 with Pixel 3 (Android 11), and Samsung Note 10(Android 10).