We currently have https://wiki.mozilla.org/Tiles/Technical_Documentation I've been in the process of converting MediaWiki syntax into reStructuredText at browser/docs/DirectoryLinksProvider.rst. bsmedberg, does this need to be uplifted to the appropriate repository or just in mozilla-central is ok? If uplift is needed, does it need the usual approval or is it npotb?

Uploaded the output: https://people.mozilla.org/~elee/docs/DirectoryLinksProvider.html

Comment on attachment 8595770 [details] [diff] [review] v1 >+++ b/browser/docs/DirectoryLinksProvider.rst >@@ -0,0 +1,118 @@ >+================================== >+DirectoryLinksProvider data format >+================================== This is both architecture and data. I suggest "Directory Links Architecture and Data Formats" >+DirectoryLinksProvider provides potentially remotely hosted JSON links data to >+be combined with local browser data to then be displayed in Firefox as Tiles on >+the New Tab Page. This module also handles reporting back data about the tiles. I suggest starting this out describing the general purpose and definitions: "Directory links are enhancements to the new-tab experience. There are three kinds of links:" and then the list. The details about DirectoryLinksProvider and JSON and whatnot can come after that. >+To potentially show any of Default, Suggested, or Enhanced Tiles; at most once >+per 24 hours, a JSON file is downloaded containing lists of tiles with data to Is there name for this file, e.g. "the directory source"? That way below when you document the format it's clear which file you're documenting. >+- Default Tiles fill in if there aren't enough history tiles being shown >+- Suggested Tiles are shown if the user's top sites match any of the triggering >+ criteria "the triggering criteria" seems vague here. Please make this specific "if the user's top sites match a category" >+For the data source and ping endpoints, the default preference values point to >+Mozilla key-pinned servers with encryption. No cookies are used. Is "no cookies" enforced technically, or is that just a promise that the server won't set cookies? Can you please list the actual endpoint values in this doc? >+ >+ >+Preferences >+=========== >+ >+There are two main preferences that control downloading links and reporting >+metrics. >+ >+``browser.newtabpage.directory.source`` >+--------------------------------------- >+ >+This endpoint tells Firefox where to download links as a GET request. It should >+return JSON of the appropriate format containing the relevant links data. The >+value can be a data URI, e.g., an empty JSON object effectively turns off remote >+downloading: ``data:text/plain,{}`` >+ >+Firefox caches the JSON file on disk after downloading it. On init and after >+sending a ping, Firefox checks if it's been at least 24 hours since downloading. >+ >+The preference value will have %LOCALE% and %CHANNEL% replaced by the >+appropriate values for the build of Firefox. >+ >+``browser.newtabpage.directory.ping`` >+------------------------------------- >+ >+This endpoint tells Firefox where to report Tiles metrics as a POST request. The >+data is sent as a JSON blob. Setting it to empty effectively turns off reporting >+of Tiles data. >+ >+Pings are sent when the user opens a new tab page as well as when the user >+clicks on tiles. If suggestions are turned off, no pings are sent. This paragraph doesn't belong here. This is a policy point that should be below. Also, this is presumably governed by one or more prefs exposed via the new-tab settings menu. Are those documented? >+ >+A path segment will be appended to the endpoint of "view" or "click" depending >+on the type of ping. This isn't clear. Let's use real example data here. >+Source JSON Format >+================== >+ >+Firefox expects links data in a JSON object with top level keys each providing >+an array of tile objects. The keys correspond to the different types of links: >+``directory``, ``suggested``, and ``enhanced``. >+ >+Link Object >+----------- >+ >+Each link object has various values that Firefox uses to display a tile: >+ >+- ``imageURI`` - string url for the tile image to show. Only https and data URIs >+ are allowed. >+- ``enhancedImageURI`` - string url for the image to be shown before the user >+ hovers. Only https and data URIs are allowed. My understanding was that these images were always provided by Mozilla. Is that true, and is it enforced technically? Related, but probably not for this section: does the image fetch also have no cookies? >+- ``bgColor`` - string css color for additional fill background color. >+- ``directoryId`` - id of the tile to be used when reporting back s/when reporting back/during ping reporting/ >+Suggested Link Object Extras >+---------------------------- >+ >+A suggested link has additional values: >+ >+- ``frecent_sites`` - array of strings of the sites that can trigger showing a >+ Suggested Tile if the user has the site in one of the top 100 most-frecent >+ pages. I thought this was removed/changed as part of building categories into the browser!? >+Ping JSON Format >+================ >+ >+Firefox reports back an action and the state of tiles on the new tab page based >+on the user opening a new tab or clicking a tile. The top level keys of the ping: >+ >+- ``locale`` - string locale of the Firefox build >+- ``tiles`` - array of tiles ping objects >+ >+An additional key at the top level indicates which action triggered the ping as >+well as the corresponding index into the tiles array of which tile triggered the >+action. Valid actions: block, click, pin, sponsored, sponsored_link, unpin, >+view. I don't understand this sentence. Does this mean "action": "block" or does it actually mean "block": <id> or something else? Can you maybe document this more by example like https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/toolkit/components/telemetry/telemetry/common-ping.html with inline comments? >+ >+Tiles Ping Object >+----------------- >+ >+Each tile of the new tab page is reported back as part of the ping with these >+values: >+ >+- ``id`` - id that was provided as part of the downloaded link object; undefined >+ if the tile was created from user behavior, e.g., visiting pages undefined is not JSON. Do you mean null? Can we just remove the id field in that case? Do we report the ID for enhanced tiles? I thought we weren't doing that for history-leakage reasons. >+- ``url`` - empty string if it's an enhanced tile; undefined otherwise undefined again. Also this is weird. Is this a holdover from when we were going to report the URL from enhanced tiles and decided not to? Can we make this a separate "enhanced": true property and get rid of the url footgun here? This doc doesn't cover any of the things I expected related to having categories built into the browser for related tiles. Is that going in a different doc?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > This is both architecture and data. I suggest "Directory Links Architecture > and Data Formats" Done. > I suggest starting this out describing the general purpose and definitions: Done. > Is there name for this file, e.g. "the directory source"? That way below > when you document the format it's clear which file you're documenting. No name, but I've updated the doc to refer to it as "directory source file." > "the triggering criteria" seems vague here. Please make this specific "if > the user's top sites match a category" Added what it means with an example. > Is "no cookies" enforced technically, or is that just a promise that the > server won't set cookies? It's our servers that don't set cookies. I noted that it isn't enforced. > Can you please list the actual endpoint values in this doc? Added. > >+Pings are sent when the user opens a new tab page as well as when the user > >+clicks on tiles. If suggestions are turned off, no pings are sent. I added a whole section for "Data Flow" that includes these policy points and explanation of logic. > >+A path segment will be appended to the endpoint of "view" or "click" depending > >+on the type of ping. > This isn't clear. Let's use real example data here. Added examples for both prefs. > >+- ``imageURI`` - string url for the tile image to show. Only https and data URIs > >+ are allowed. > >+- ``enhancedImageURI`` - string url for the image to be shown before the user > >+ hovers. Only https and data URIs are allowed. > My understanding was that these images were always provided by Mozilla. Is > that true, and is it enforced technically? I added an example directory source file that shows imageURIs pointing to https://tiles.cdn.mozilla.net/images/... Firefox does indeed filter out tiles that have images pointing to something not https or data. > Related, but probably not for this section: does the image fetch also have > no cookies? I added a note in the "Data Flow" section. > s/when reporting back/during ping reporting/ Done. > >+- ``frecent_sites`` - array of strings of the sites that can trigger showing a > >+ Suggested Tile if the user has the site in one of the top 100 most-frecent > >+ pages. > I thought this was removed/changed as part of building categories into the > browser!? I added a note about how only preapproved hardcoded array of strings are allowed. > >+An additional key at the top level indicates which action triggered the ping as > >+well as the corresponding index into the tiles array of which tile triggered the > >+action. Valid actions: block, click, pin, sponsored, sponsored_link, unpin, > >+view. > I don't understand this sentence. I reworded this and added an inline example. > >+- ``id`` - id that was provided as part of the downloaded link object; undefined > undefined is not JSON. Do you mean null? Can we just remove the id field in > that case? I meant undefined and clarified that undefined ends up not being encoded in JSON. I also added an example ping. > Do we report the ID for enhanced tiles? I thought we weren't doing that for > history-leakage reasons. We are reporting ids for all served tiles including enhanced history tiles. > >+- ``url`` - empty string if it's an enhanced tile; undefined otherwise > undefined again. Also this is weird. Is this a holdover from when we were > going to report the URL from enhanced tiles and decided not to? Can we make > this a separate "enhanced": true property and get rid of the url footgun > here? This is indeed from enhanced tiles would have sent back the urls and was changed to send empty string instead. Filed bug 1157450. > This doc doesn't cover any of the things I expected related to having > categories built into the browser for related tiles. Is that going in a > different doc? What additional explanation are you looking for? I added a note similar to "Only https and data URIs are allowed." but "Only preapproved array of strings that are hardcoded into Firefox code are allowed."

Comment on attachment 8595770 [details] [diff] [review] v1 Reuploaded the output: https://people.mozilla.org/~elee/docs/DirectoryLinksProvider.html

Comment on attachment 8596188 [details] [diff] [review] v2 >diff --git a/browser/docs/DirectoryLinksProvider.rst b/browser/docs/DirectoryLinksProvider.rst >+As the new tab page is rendered, any images for tiles are downloaded if not >+already cached. The default servers hosting the images are Mozilla CDN that >+don't use cookies: https://tiles.cdn.mozilla.net/ I filed bug 1157810 since I think we should enforce this technically. The doc here is correct and should land as-is until we implement the stronger restriction. >+Suggested Link Object Extras >+---------------------------- >+ >+A suggested link has additional values: >+ >+- ``frecent_sites`` - array of strings of the sites that can trigger showing a >+ Suggested Tile if the user has the site in one of the top 100 most-frecent >+ pages. Only preapproved array of strings that are hardcoded into Firefox code >+ are allowed. Need the complete details on this. Where are the lists of preapproved strings located in the code? Also please document the criteria that we are currently using to select sets of sites. We can change the docs later. >+- ``id`` - id that was provided as part of the downloaded link object (for all >+ types of links: directory, suggested, enhanced); undefined if the tile was >+ created from user behavior, e.g., visiting pages I really don't like the use of "undefined" here. We're describing a JSON payload, and so in fact when you say "undefined" you mean "not present". Also I filed bug 1157832 because I think the fact that enhanced tiles can profile multiple top sites is a privacy concern and doesn't match our privacy promise about whether/how a remote Mozilla server could collect history data. But again, you are correctly documenting the current behavior so we shouldn't change the docs here.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #5) > >+- ``frecent_sites`` - array of strings of the sites that can trigger showing a > Need the complete details on this. Pointed to DirectoryLinksProvider module and provided the criteria and some context for why. > I really don't like the use of "undefined" here. We're describing a JSON > payload, and so in fact when you say "undefined" you mean "not present". I prefaced the section with a note that some or none of these optional values and switched "undefined"s to "not present"s.

Ed, since this bug is dependent on bug 1152145, does that also need uplift to 39? Thanks.

(In reply to Liz Henry (:lizzard) from comment #9) > Ed, since this bug is dependent on bug 1152145, does that also need uplift > to 39? Thanks. This patch has no impact on build. The dependency is so that the docs match up with what code is in the repository.

Comment on attachment 8599142 [details] [diff] [review] v3 Approved for uplift to aurora as requested in bug 1156876.