Closed Bug 1143745 Opened 5 years ago Closed 5 years ago

Update the way Firefox reads directoryLinks.json sent from the server for the tiles v3 endpoint

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
Points:
5

Tracking

()

RESOLVED FIXED
Firefox 39
Iteration:
39.3 - 30 Mar
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: emtwo, Assigned: emtwo)

References

Details

(Whiteboard: .?)

Attachments

(1 file)

Bug 1134550 will create a tiles v3 endpoint. This will require a change in Firefox to point to the new endpoint and to read/handle the new format:

{ "related": [{"title": "tile", "type": "affiliate"},
  "directory": [{...},...] }
Whiteboard: .?
oyiptong has a good point that Firefox 38 assumes the content is all affiliate suggested tiles, but for Firefox 39 we'll want to serve sponsored suggested tiles -- probably using the same v3 endpoint. This means for Firefox 38, we'll want to explicitly only allow type=affiliate to avoid accidentally showing sponsored tiles without the correct messaging.
QA Contact: msamuel
Assignee: nobody → msamuel
QA Contact: msamuel
Comment on attachment 8582567 [details] [diff] [review]
v1: Update the way Firefox reads directoryLinks.json sent from the server for the tiles v3 endpoint

Review of attachment 8582567 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/DirectoryLinksProvider.jsm
@@ +299,3 @@
>     */
>    _readDirectoryLinksFile: function DirectoryLinksProvider_readDirectoryLinksFile() {
> +    let emptyOutput = {'directory': [], 'suggested': []};

Nit: Don't need the quotes around the keys, here and below.
Attachment #8582567 - Flags: review?(adw) → review+
https://hg.mozilla.org/mozilla-central/rev/ca81732ae419
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Iteration: --- → 39.3 - 30 Mar
Points: --- → 5
>-    for (let relatedSite of link.suggested) {
>+    for (let relatedSite of link.frecent_sites) {
In the process of updating tests for bug 1140496, I ran into an exception here because I didn't include frecent_sites in the tile object. We should probably skip over invalid entries. We had a previous bug where we expected an array as the value of "en-US" key, but when the server accidentally served { en-US: { en-US: [..] } }, it completely broke new tab pages for Firefox users.

Although I believe an exception in the insides of getLinks will trigger the failure path of the promise resulting in reportError and returning [].
Blocks: 1148859
Blocks: 1155443
No longer blocks: 1155443
Blocks: 1168631
You need to log in before you can comment on or make changes to this bug.