Closed
Bug 897359
Opened 11 years ago
Closed 10 years ago
Remove unimplemented method in nsCrypto
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: allstars.chh, Assigned: anujagarwal464)
Details
(Keywords: dev-doc-needed, site-compat)
Attachments
(1 file, 2 obsolete files)
8.53 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
In Bug 883741, comment 61 BZ Suggested we'd remove those unimplemented methods in nsCrypto. (including popChallengeResponse, random, and disableRightClick).
Assignee | ||
Comment 1•10 years ago
|
||
I would like to work on this bug, could you update the description.
Flags: needinfo?(allstars.chh)
Reporter | ||
Comment 2•10 years ago
|
||
I haven't started working on this bug, feel free to take it.
Assignee: allstars.chh → nobody
Flags: needinfo?(allstars.chh)
Assignee | ||
Comment 3•10 years ago
|
||
Assignee: nobody → anujagarwal464
Attachment #8407450 -
Flags: feedback?(allstars.chh)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8407450 [details] [diff] [review] bug897359.diff Review of attachment 8407450 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I am not a peer in either DOM nor Security. Can you find someone else to review your patch? Thanks
Attachment #8407450 -
Flags: feedback?(allstars.chh)
Assignee | ||
Updated•10 years ago
|
Attachment #8407450 -
Flags: review?(bzbarsky)
Comment 5•10 years ago
|
||
Comment on attachment 8407450 [details] [diff] [review] bug897359.diff Thank you for working on this! Have you compiled this patch? Have you run the tests in dom/tests/mochitest/crypto ? I'm guessing the answer is "no", for both of those..... Please do that and fix the issues it catches?
Attachment #8407450 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8407450 [details] [diff] [review] bug897359.diff Build was successful and all tests in dom/tests/mochitest/crypto passed. (In reply to Boris Zbarsky [:bz] from comment #5) > Comment on attachment 8407450 [details] [diff] [review] > bug897359.diff > > Thank you for working on this! > > Have you compiled this patch? Have you run the tests in > dom/tests/mochitest/crypto ? I'm guessing the answer is "no", for both of > those..... Please do that and fix the issues it catches?
Attachment #8407450 -
Flags: review?(bzbarsky)
Comment 7•10 years ago
|
||
Comment on attachment 8407450 [details] [diff] [review] bug897359.diff Oh, I see, because these are in the superclass. So this is fine as far as it goes. r=me on this part. But what I was actually asking for in bug 883741 is to remove these pointless methods completely. Not just from nsCrypto, but also from Crypto and from Crypto.webidl.
Attachment #8407450 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•10 years ago
|
||
When I compile with this patch I'm getting following error error C3668: 'nsCrypto::PopChallengeResponse' : method with override specifier 'override' did not override any base class methods
Attachment #8408399 -
Flags: feedback?(bzbarsky)
Comment 9•10 years ago
|
||
Comment on attachment 8408399 [details] [diff] [review] bug897359.diff incomplete That's odd, since you're removing nsCrypto::PopChallengeResponse, no?
Attachment #8408399 -
Flags: feedback?(bzbarsky) → feedback+
Comment 10•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=84a9a3ae8068 for that last patch. The reason for the compile failure in comment 8 was doing a local build in dom/, which didn't export the modified header.
Assignee | ||
Comment 11•10 years ago
|
||
Try build was successful.
Attachment #8408399 -
Attachment is obsolete: true
Attachment #8408770 -
Flags: review?(bzbarsky)
Comment 12•10 years ago
|
||
Comment on attachment 8408770 [details] [diff] [review] bug897359.diff -final r=me
Attachment #8408770 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8407450 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5bffd7d60c5
Flags: in-testsuite+
Keywords: checkin-needed
Comment 14•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e5bffd7d60c5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•10 years ago
|
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•