Closed Bug 1233628 Opened 9 years ago Closed 9 years ago

handling {} for clear-origin-data in PushService.jsm

Categories

(Core :: DOM: Notifications, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID
Tracking Status
firefox46 --- affected

People

(Reporter: allstars.chh, Assigned: allstars.chh)

References

Details

Attachments

(1 file)

See Honza's comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1233136#c0 "PushService [1], bug 1170115, that seems to do the right thing, except that empty aData string is simply ignored (the code doesn't delete anything)"
Attached patch Patch (deleted) — Splinter Review
Attachment #8699852 - Flags: review?(kcambridge)
Comment on attachment 8699852 [details] [diff] [review] Patch Review of attachment 8699852 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch! r=me with the two issues fixed. ::: dom/push/PushService.jsm @@ -295,1 @@ > let pattern = JSON.parse(data); This doesn't seem quite right. `JSON.parse("")` will throw a syntax error. How about `data ? JSON.parse(data) : {}`? ::: dom/push/test/xpcshell/test_clear_origin_data.js @@ +8,5 @@ > const userAgentID = 'bd744428-f125-436a-b6d0-dd0c9845837f'; > > let clearForPattern = Task.async(function* (testRecords, pattern) { > let patternString = JSON.stringify(pattern); > + yield PushService.observe(null, 'clear-origin-data', patternString); Please change `observe` to return the promise. Otherwise, this might race.
Attachment #8699852 - Flags: review?(kcambridge) → feedback+
Oh your're right, I made a mistake, I thought JSON.stringify("{}") will become "" but it should be "{}", so the original code should work already, I'll go back to bug 1233136 to confirm with Honza again. Thanks
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: