1212897 - Pass strings instead of objects from remote page to content process

Status: RESOLVED WONTFIX

(Firefox :: New Tab Page, defect)

Comment 1

[Olivier Yiptong [:oyiptong]]

Attachment: MozReview Request: other changes reviewed elsewhere

other changes reviewed elsewhere

Comment 2

[Olivier Yiptong [:oyiptong]]

Attachment: MozReview Request: Bug 1212897 - Pass strings instead of objects from remote page to content process r?bholley

Bug 1212897 - Pass strings instead of objects from remote page to content process r?bholley

Comment 3

[Olivier Yiptong [:oyiptong]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bdea376ad1d

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8671588 [details]
MozReview Request: Bug 1212897 - Pass strings instead of objects from remote page to content process r?bholley

https://reviewboard.mozilla.org/r/21615/#review19423

::: browser/base/content/remote-newtab/newTab.js:70
(Diff revision 1)
> -        // passed on to main process, in RemoteAboutNewTab.jsm
> +        if(typeof data !== "string"){

Nit: space before paren.

::: browser/base/content/remote-newtab/newTab.js:74
(Diff revision 1)
> +        try{

Nit: Space before brace.

::: browser/base/content/remote-newtab/newTab.js:127
(Diff revision 1)
>        name: "NewTab:State",

Don't you need to do something to JSONify this?

Comment 5

[marcos]

Attachment: 0001-Bug-1212897-Pass-strings-instead-of-objects-from-rem.patch

Bobby, for this patch, we just want to handle the messages coming up to the browser from the content. Is that ok? 

All messages will be going away once bug 1204983 lands, as we will replace all messages with a WebIDL interface.

Comment 6

[Bobby Holley (:bholley)]

Comment on attachment 8673245 [details] [diff] [review]
0001-Bug-1212897-Pass-strings-instead-of-objects-from-rem.patch

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

My point is that this patch doesn't look correct on its own because it's changing the receiver to expect JSON, but doesn't appear to change the sender anywhere.

It may be that the sender is in some other code that isn't in-tree or somesuch. Regardless, I don't think I'm the right reviewer here. f+ on using JSON.

Comment 7

[Olivier Yiptong [:oyiptong]]

we will not serialize the data as we'll be moving to a web IDL method before releasing to the public