Closed Bug 872377 Opened 12 years ago Closed 11 years ago

Restore "toJSON" to list of reserved webidl keywords

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla25

People

(Reporter: jib, Assigned: almasry.mina)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Patch, removed TODO comment
(deleted), patch
Details | Diff | Splinter Review
Allow toJSON identifier to be used again
(deleted), patch
peterv
: review+
Details | Diff | Splinter Review
Once 863402 is fixed, remove attachment 749583 [details] [diff] [review] workaround.
Assignee: nobody → almasry.mina
Attached patch resever-toJSON.patch (obsolete) (deleted) — — Splinter Review
Here is a patch that removes the toJSON implementation and relies on the jsonifier just added. Test included.
Attachment #781536 - Flags: review?(bzbarsky)
Comment on attachment 781536 [details] [diff] [review] resever-toJSON.patch Lovely. r=me
Attachment #781536 - Flags: review?(bzbarsky) → review+
Keywords: checkin-needed
Maybe I shouldn't mark this as checkin-needed until the jsonifier patch lands...
Keywords: checkin-needed
Keywords: checkin-needed
Attached patch Patch, removed TODO comment (deleted) — — Splinter Review
I removed the TODO comment that had to do with this bug.
Attachment #781536 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/ca0c8e910426
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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?
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.
(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.
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.
Attached patch Allow toJSON in WebIDL interfaces again. (obsolete) (deleted) — — Splinter Review
Attachment #821093 - Attachment description: aeiou → Allow toJSON in WebIDL interfaces again.
Attachment #821093 - Attachment is patch: true
Attachment #821093 - Flags: review?(continuation)
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.
Attachment #821093 - Flags: review?(continuation) → review?(peterv)
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.
Attachment #821093 - Attachment is obsolete: true
Attachment #821093 - Flags: review?(peterv)
Attachment #821265 - Flags: review?(peterv)
Blocks: 929492
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.").
Attachment #821265 - Flags: review?(peterv) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: