Closed
Bug 944890
Opened 11 years ago
Closed 11 years ago
B2G SMS & MMS: remove convertTimeToInt(...) which is no longer needed
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: airpingu, Assigned: airpingu)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
At bug 939302, we interpret all the timestamps as long-long numbers instead of Date objects. Eventually, all the timestamp values in the DOM, IPDL, SMS/MMS internal codes can just be passed around as long-long numbers. We should be able to remove convertTimeToInt(...) which is no longer needed anymore.
Assignee | ||
Updated•11 years ago
|
Severity: critical → minor
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #1) > Why is this security-sensitive? It shouldn't be. Sorry I cloned that bug without unchecking that flag but I'm still not able to uncheck that option now. Could anyone please help me? Thanks!
Updated•11 years ago
|
Group: core-security
Assignee | ||
Updated•11 years ago
|
Flags: in-testsuite+
Comment 6•11 years ago
|
||
Comment on attachment 8393383 [details] [diff] [review] Part 1, implementation Review of attachment 8393383 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, can't remember which bug, but I remember I was once told not to align argument names.
Attachment #8393383 -
Flags: review?(vyang)
Comment 7•11 years ago
|
||
Comment on attachment 8393384 [details] [diff] [review] Part 2, test cases Review of attachment 8393384 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/tests/test_smsservice_createsmsmessage.js @@ -145,5 @@ > > /** > - * Test supplying the timestamp as a Date object. > - */ > -add_test(function test_timestamp_date() { Please rewrite this as what we have in test_invalid_timestamp_float. @@ -168,5 @@ > - > -/** > - * Test that a floating point number for the timestamp is not allowed. > - */ > -add_test(function test_invalid_timestamp_float() { Please keep this and replace |new Date()| with |Date.now()|. We still want to verify acceptable argument types in this script.
Attachment #8393384 -
Flags: review?(vyang)
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7) > > -add_test(function test_invalid_timestamp_float() { > > Please keep this and replace |new Date()| with |Date.now()|. We still want > to verify acceptable argument types in this script. Since the float/null/undefined will be automatically casted to valid unsigned long long value, I'd prefer removing these tests.
Assignee | ||
Comment 9•11 years ago
|
||
> Since the float/null/undefined will be automatically casted to valid
> unsigned long long value, I'd prefer removing these tests.
Random object like {} will also implicitly be casted to integer.
Assignee | ||
Comment 10•11 years ago
|
||
Hi Vicamo, note that I'd still prefer align the parameters for IDL and IPDL because they are just for generating codes. Also, I saw lots of examples in WebIDL/IDL aligning parameters in this way so I think it's OK to do so.
Attachment #8393383 -
Attachment is obsolete: true
Attachment #8393423 -
Flags: review?(vyang)
Assignee | ||
Comment 11•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e5a35e95c210
Attachment #8393384 -
Attachment is obsolete: true
Attachment #8393424 -
Flags: review?(vyang)
Comment 12•11 years ago
|
||
Comment on attachment 8393423 [details] [diff] [review] Part 1, implementation, V2 Review of attachment 8393423 [details] [diff] [review]: ----------------------------------------------------------------- Thank you :)
Attachment #8393423 -
Flags: review?(vyang) → review+
Updated•11 years ago
|
Attachment #8393424 -
Flags: review?(vyang) → review+
Assignee | ||
Comment 13•11 years ago
|
||
remote: https://hg.mozilla.org/integration/b2g-inbound/rev/9023afb674fa remote: https://hg.mozilla.org/integration/b2g-inbound/rev/9b44086c1d1c
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9023afb674fa https://hg.mozilla.org/mozilla-central/rev/9b44086c1d1c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•