778326 - Create Permission.txt that explains permisson-names and purpose

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: v1

Before I go through all APIs I want some feedback how this should look like. 
Maybe like:

Permission Name / API name
1) Short description
2) Possible types

contacts / Contacts API
1) Add/Read/Modify the device contacts address book and read contacts from SIM card.
2) contacts {read, write, create}

The types come from the discussion about the new format of .webapp files.
For example:
  "permissions": {
    "mozbrowser": {
      description: "...",
      access: "read"
    },
  },

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 647366 [details] [diff] [review]
v1

This is pretty epic bikeshedding, but how about

alarm{read, write, create}  // NB: is it "alarm-read" or "alarmread"?
Alarm API
Schedule a notification, or for an application to be started, at a specific time.

contacts{read, write, create, delete}
Contacts API
Add/Read/Modify the device contacts address book and read contacts from SIM card.

Comment 3

[Anant Narayanan [:anant]]

FWIW, I think using an explicit "access" field is better than rolling the type of access into the permission name itself.

Comment 4

[[:fabrice] Fabrice Desré]

(In reply to Gregor Wagner [:gwagner] from comment #1)

> The types come from the discussion about the new format of .webapp files.
> For example:
>   "permissions": {
>     "mozbrowser": {
>       description: "...",
>       access: "read"
>     },
>   },

I'm not sure on which ML this was discussed, but I think the description should not be bundled with the permission itself, since it is localizable but we don't want localizers to change the permission set itself.

So maybe something like:

permissions: {
  mozbrowser: "read",
  contacts: "read,write"
}

permissions_usage: {
  mozbrowser: "To browse the web",
  contacts: "To read and write your contacts"
}

fr_FR: {
  permissions_usage: {
    mozbrowser: "Pour naviguer sur le grand internet",
    contacts: "Pour lire et écrire dans votre carnet d'adresses"
  }
}

Comment 5

[Anant Narayanan [:anant]]

Good point, Fabrice! I would like us to stay consistent with the existing mechanism for localization in the manifest, however. It's okay to include the description with the permission itself, as long as it is clear what the default_locale is. (For example: http://mozilla.github.com/webapps-spec/#example).

This translates to a manifest that looks something like:

{
...
name: "MozillaBall",
description": "Exciting Open Web development action!",
permissions: {
  "mozbrowser": {
    description: "To browse the web"
  },
  "contacts": {
    description: "To read and write your contacts",
    access: "read,write"
  }
},
locales: {
  fr_FR: {
    name: "MozillaBalle",
    description: "Passionnant d'action de développement Open Web!",
    permissions: {
      mozbrowser: "Pour naviguer sur le grand internet",
      contacts: "Pour lire et écrire dans votre carnet d'adresses"
    }
  }
},
default_locale: en,
...
}

Does this look reasonable?

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to Anant Narayanan [:anant] from comment #5)

> 
> Does this look reasonable?

I'm not a big fan of having the default locale and other locales use a different property ("description" vs nothing) but I can live with that.

Comment 7

[Anant Narayanan [:anant]]

(In reply to Fabrice Desré [:fabrice] from comment #6)
> I'm not a big fan of having the default locale and other locales use a
> different property ("description" vs nothing) but I can live with that.

Oh, sorry that was a mistake on my part; according to the spec, the properties under a locale must match the top-level hierarchy, so that section should actually read as:

fr_FR: {
  permissions: {
    mozbrowser: {
      description: "Pour naviguer sur le grand internet"
    }
  }
}

Comment 8

[[:fabrice] Fabrice Desré]

Ok, that looks perfect to me!

Comment 9

[Gregor Wagner [:gwagner]]

(In reply to Anant Narayanan [:anant] from comment #7)
> (In reply to Fabrice Desré [:fabrice] from comment #6)
> > I'm not a big fan of having the default locale and other locales use a
> > different property ("description" vs nothing) but I can live with that.
> 
> Oh, sorry that was a mistake on my part; according to the spec, the
> properties under a locale must match the top-level hierarchy, so that
> section should actually read as:
> 
> fr_FR: {
>   permissions: {
>     mozbrowser: {
>       description: "Pour naviguer sur le grand internet"
>     }
>   }
> }

Right, that's how Jonas convinced me yesterday.

Comment 10

[Gregor Wagner [:gwagner]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Comment on attachment 647366 [details] [diff] [review]
> v1
> 
> This is pretty epic bikeshedding, but how about

That's why I asked right when I started so bikeshedding doesn't matter at all.

> 
> alarm{read, write, create}  // NB: is it "alarm-read" or "alarmread"?
> Alarm API
> Schedule a notification, or for an application to be started, at a specific
> time.
> 
> contacts{read, write, create, delete}
> Contacts API
> Add/Read/Modify the device contacts address book and read contacts from SIM
> card.

Looks good to me!

Comment 11

[Gregor Wagner [:gwagner]]

Attachment: permissions

Comment 12

[Gregor Wagner [:gwagner]]

Attachment: Permissions

First version.

Comment 13

[David Dahl :ddahl]

(In reply to Anant Narayanan [:anant] from comment #5)
> ...
> name: "MozillaBall",
> description": "Exciting Open Web development action!",
> permissions: {
>   "mozbrowser": {
>     description: "To browse the web"
>   },
>   "contacts": {
>     description: "To read and write your contacts",
>     access: "read,write"
>   }
> },

I am thinking that:

access: "read,write" 

might be better as: 

access: {read: true, write: true} 

or for brevity:

access: "rw"

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

There are only 4 different values that makes sense for the access field:

* read
* create
* read+create
* read+write+create

So I don't think we'll want to have an array like ["read", "write"], or flags like { read: true, write: true }. Either of those options just lead to too much ability to set invalid combinations. So I'm suggesting that we just allow 4 different string values:

access: "readonly"
access: "createonly"
access: "readcreate"
access: "readwrite"

Another nice thing with this is that "readonly" and "readwrite" matches what's used in a lot of other contexts (read-only memory, read-write web)

Comment 15

[Anant Narayanan [:anant]]

(In reply to Jonas Sicking (:sicking) from comment #14)
> access: "readonly"
> access: "createonly"
> access: "readcreate"
> access: "readwrite"
> 
> Another nice thing with this is that "readonly" and "readwrite" matches
> what's used in a lot of other contexts (read-only memory, read-write web)

+1, since there are only 4 valid combinations, this makes the most sense.

Comment 16

[Andreas Gal :gal]

Why do you want to block on this?

Comment 17

[Lucas Adamski [:ladamski]]

Need this to codify the permission sets and syntax.  Necessary for the permission model AFAIK (otherwise we should stop working on it for now).

Comment 18

[Andreas Gal :gal]

Sure, agree with that, but this seems to be documentation, not a code bug.

Comment 19

[Lucas Adamski [:ladamski]]

The output is a document but the work is defining the exact permissions syntax and grammar.  Seems like we need to figure that out in order to use permissions.

Comment 20

[Andreas Gal :gal]

Ok, sounds good.

Comment 21

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 22

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dbf4f6c5bf32

Comment 23

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/dbf4f6c5bf32

Comment 24

[Martijn Wargers (dead)]

http://mxr.mozilla.org/mozilla-central/source/extensions/cookie/Permission.txt#98
This says:
98 network-http
99 {}
100 old name: systemXHR

But I can't find anything 'network-http' in the source, but I can find 'systemXHR'.