872377 - Restore "toJSON" to list of reserved webidl keywords

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Once 863402 is fixed, remove attachment 749583 [details] [diff] [review] workaround.

Comment 1

[Mina Almasry]

Attachment: resever-toJSON.patch

Here is a patch that removes the toJSON implementation and relies on the jsonifier just added. Test included.

Comment 2

[Mina Almasry]

Try push:

https://tbpl.mozilla.org/?tree=Try&rev=ef9b9eae9192

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 781536 [details] [diff] [review]
resever-toJSON.patch

Lovely.  r=me

Comment 4

[Mina Almasry]

Maybe I shouldn't mark this as checkin-needed until the jsonifier patch lands...

Comment 5

[Mina Almasry]

Attachment: Patch, removed TODO comment

I removed the TODO comment that had to do with this bug.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Reuben Morais [:reuben]]

How does |jsonifier| replace toJSON? jsonifier enumerates all non-static and serializable attributes in the interface. toJSON lets me do whatever I want (in my case, use my own representation for |object| attributes that I know are always arrays). Can I control the behavior of jsonifier somehow?

Comment 9

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

It doesn't I'm sure. But it replaces the workaround I had for not having serializer support (Bug 863402) in PeerConnection.js in comment 0.

Comment 10

[Reuben Morais [:reuben]]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #9)
> It doesn't I'm sure. But it replaces the workaround I had for not having
> serializer support (Bug 863402) in PeerConnection.js in comment 0.

Okay. In that case, can I back this out? Contacts needs the same workaround.

Comment 11

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Covered on IRC, reuben will "revert" only the change to WebIDL.py, since he still needs a custom toJSON() method to serialize his array stuff, which webidl doesn't handle yet.

Comment 12

[Reuben Morais [:reuben]]

Attachment: Allow toJSON in WebIDL interfaces again.

Comment 13

[Andrew McCreight [:mccr8]]

Comment on attachment 821093 [details] [diff] [review]
Allow toJSON in WebIDL interfaces again.

A DOM peer should review this, and I don't know anything about this issue in any event.  Maybe peterv could look at it.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 821093 [details] [diff] [review]
Allow toJSON in WebIDL interfaces again.

Please restore the comment about how per-spec toJSON needs to be in this list.

Comment 15

[Reuben Morais [:reuben]]

Attachment: Allow toJSON identifier to be used again

Comment 16

[Peter Van der Beken [:peterv]]

Comment on attachment 821265 [details] [diff] [review]
Allow toJSON identifier to be used again

First of all, open a new bug for this patch. This bug is not about allowing toJSON, and piling on is not the right thing to do. Second, you should probably describe in the comment why we need it ("We sometimes need custom serialization behaviour, so allow toJSON for now.").