Remove browserFlag attributes in Permission API methods

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
6 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 742739 [details] [diff] [review]
Patch

I've been asked to review the Permission API documentation and the browserFlag parameter is very confusing for the API consumer: this is an implementation detailed exposed to developers that don't need to know about it and shouldn't even bother. A good example that this attribute is useless: no one is actually using it. Neither in Gaia nor in Gecko tests.

We need methods on the Browser API to handle permission handling for websites.
Attachment #742739 - Flags: superreview?(jonas)
Attachment #742739 - Flags: review?(anygregor)
(Assignee)

Comment 1

6 years ago
FWIW, we will have to update Gaia after this patch but the changes should be trivial.
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed
Attachment #742739 - Flags: review?(anygregor) → review+
(In reply to Mounir Lamouri (:mounir) from comment #1)
> FWIW, we will have to update Gaia after this patch but the changes should be
> trivial.

Well but it also means that v1 gaia doesn't work with trunk gecko any more.
(Assignee)

Comment 4

6 years ago
(In reply to Gregor Wagner [:gwagner] from comment #3)
> (In reply to Mounir Lamouri (:mounir) from comment #1)
> > FWIW, we will have to update Gaia after this patch but the changes should be
> > trivial.
> 
> Well but it also means that v1 gaia doesn't work with trunk gecko any more.

Is currently? I do not believe that we should make this a requirement, otherwise we will never get things done.
(Assignee)

Comment 5

6 years ago
Jonas, any ETA on this sr?
Flags: needinfo?(jonas)
This means that the permission app won't be able to modify permissions of bookmark-to-homescreen "apps".

I don't understand what you mean by that the browserFlag is an "implementation detail". Browser content certainly is a concept that consumers of the permission API needs to be aware of.
Flags: needinfo?(jonas)
(Assignee)

Comment 7

6 years ago
(In reply to Jonas Sicking (:sicking) from comment #6)
> This means that the permission app won't be able to modify permissions of
> bookmark-to-homescreen "apps".

Is it running with browserFlag=true? If yes, it is currently not doing it.

> I don't understand what you mean by that the browserFlag is an
> "implementation detail". Browser content certainly is a concept that
> consumers of the permission API needs to be aware of.

I mean that the "browserFlag" is an implementation detail. We should indeed differentiate web content and apps but that should be handled by the Browser API. Basically, the browser API should have its own permission API to take care of web sites permissions.
(In reply to Mounir Lamouri (:mounir) from comment #7)
> (In reply to Jonas Sicking (:sicking) from comment #6)
> > This means that the permission app won't be able to modify permissions of
> > bookmark-to-homescreen "apps".
> 
> Is it running with browserFlag=true?

Yes.

> If yes, it is currently not doing it.

I'm not sure what the two "it"s are referring to above.

The settings app currently doesn't doesn't show UI for adjusting bookmark-to-homescreen applications, no.

> > I don't understand what you mean by that the browserFlag is an
> > "implementation detail". Browser content certainly is a concept that
> > consumers of the permission API needs to be aware of.
> 
> I mean that the "browserFlag" is an implementation detail. We should indeed
> differentiate web content and apps but that should be handled by the Browser
> API. Basically, the browser API should have its own permission API to take
> care of web sites permissions.

I still don't think the browserFlag is an implementation detail. But if you have suggestions for how the settings app should be able to adjust permissions of bookmark-to-homescreen apps, as well as permissions for browser content inside 3rd party browser apps, then I'm all ears.

I.e. I want the settings app to be able to show "all content that has been granted permission to read geolocation", be that 3rd party apps, or maps.google.com when displayed in the firefox app.
(Assignee)

Comment 9

6 years ago
(In reply to Jonas Sicking (:sicking) from comment #8)
> I still don't think the browserFlag is an implementation detail. But if you
> have suggestions for how the settings app should be able to adjust
> permissions of bookmark-to-homescreen apps, as well as permissions for
> browser content inside 3rd party browser apps, then I'm all ears.

The fact that we launch those bookmarked pages in a <iframe mozbrowser> but want the user to consider them as an application is an implementation detail. We could as well have fake manifest URLs and they would behave like apps as any other installed application.
Attachment #742739 - Flags: superreview?(jonas) → superreview-
If we fake a manifest for them and run them as "app context", or if we fake a manifest for them and run them as "web page context within an app" is a security/UX choice. However in either scenario the settings app needs to be able to change its security preferences.

I still maintain that the browserFlag isn't an implementation detail. It's an integral and testable part of the security model as affected by the browser API.

So I don't see how we could fix this bug without stopping supporting required use cases.
(Assignee)

Comment 11

6 years ago
I do not buy the argument that this is required but we don't use it at all. Also, if we can use a fake application or a fake browser context for bookmarked application, the fact that we use one or the other solution is then an implementation detail. If we have to standardize an API like Permissions API, it is unlikely that we will force people to use the same implementation details than our security model.In my opinion, the bits of the Permission Manager that use browserFlag=true should live in the Browser API.

Anyway, we are going into circles here...
Feel free to make proposals for how to change the browser API to fulfill the use cases we have. But until we have those proposals defined and implemented we should not remove the capabilities here.
(Assignee)

Comment 13

6 years ago
We can have two methods in the Browser API:
  DOMString getPermission(DOMString origin);
  void      setPermission(DOMString origin, DOMString permission);
(they could/should be async)
That wouldn't let the settings app enumerate the permissions given by by browser apps would it? Nor would it let us manage the permissions given to homescreen bookmarks.

Also, having to create an iframe element just to get access to permissions that has nothing to do with that the contents of that iframe is a bit iffy.
(Assignee)

Comment 15

5 years ago
I strongly disagree with Jonas but I do not see the point in keeping this bug open forever. Marking as WONTFIX.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.