Closed Bug 1212897 Opened 5 years ago Closed 5 years ago

Pass strings instead of objects from remote page to content process

Categories

(Firefox :: New Tab Page, defect)

40 Branch
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Iteration:
44.2 - Oct 19

People

(Reporter: oyiptong, Assigned: marcosc)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
Summary: Pass strings instead of objects to main process → Pass strings instead of objects from remote page to content process
other changes reviewed elsewhere
Bug 1212897 - Pass strings instead of objects from remote page to content process r?bholley
Attachment #8671588 - Flags: review?(bobbyholley)
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?
Attachment #8671588 - Flags: review?(bobbyholley)
Attachment #8671587 - Attachment is obsolete: true
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.
Attachment #8673245 - Flags: review?(bobbyholley)
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.
Attachment #8673245 - Flags: review?(bobbyholley) → feedback+
we will not serialize the data as we'll be moving to a web IDL method before releasing to the public
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.