1239116 - PlacesProvider Messaging API

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Tim Spurway [:tspurway]]

This API provides access to FX frecent history (places db) for the remote newtab page.

Comment 1

[Olivier Yiptong [:oyiptong]]

Attachment: MozReview Request: Bug 1239116 - Use devtools shared path for event-emitter r?emtwo

Review commit: https://reviewboard.mozilla.org/r/39285/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39285/

Comment 2

[Olivier Yiptong [:oyiptong]]

Attachment: MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review commit: https://reviewboard.mozilla.org/r/39287/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39287/

Comment 3

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/1-2/

Comment 4

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/2-3/

Comment 5

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/3-4/

Comment 6

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/4-5/

Comment 7

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/5-6/

Comment 8

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/6-7/

Comment 9

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39287/diff/7-8/

Comment 10

[Ricky Rosario [:rrosario, :r1cky]]

https://reviewboard.mozilla.org/r/39283/#review41163

::: browser/components/newtab/tests/browser/browser_newtabmessages.js:116
(Diff revision 8)
> +    } = PlacesUtils.history;
> +
> +    return {
> +      uri: NetUtil.newURI(`${domain}${index}`),
> +      visitDate: timeDaysAgo(daysAgo),
> +      transition: (isTyped) ? TRANSITION_TYPED : TRANSITION_LINK,

nit: in this object literal you have a trailing comma and in the one above after `let` you don't.

Comment 11

[Ricky Rosario [:rrosario, :r1cky]]

Comment on attachment 8729195 [details]
MozReview Request: Bug 1239116 - Places messages enabled for remote newtab r?rrosario

https://reviewboard.mozilla.org/r/39287/#review41189

::: browser/components/newtab/tests/browser/browser_newtabmessages.js:136
(Diff revision 8)
> +
> +  /** Test Begins **/
> +
> +  let tabOptions = {
> +    gBrowser,
> +    url: testURL

nit: there are trailing comas everywhere except here. Is there something special with `let` or should we have one here too?

Comment 12

[Olivier Yiptong [:oyiptong]]

https://reviewboard.mozilla.org/r/39287/#review41189

> nit: there are trailing comas everywhere except here. Is there something special with `let` or should we have one here too?

AFAIK `let` doesn't affect the requirement for not having trailing commas; it became valid syntax since ES5.
ES3 didn't allow for trailing commas. I'm personally indifferent about trailing commas.

Comment 13

[Pulsebot]

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

Comment 14

[Carsten Book [:Tomcat]]

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