897359 - Remove unimplemented method in nsCrypto

Status: RESOLVED FIXED

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

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

In Bug 883741, comment 61 BZ Suggested we'd remove those unimplemented methods in nsCrypto.
(including popChallengeResponse, random, and disableRightClick).

Comment 1

[Anuj Agarwal [:anujagarwal464]]

I would like to work on this bug, could you update the description.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I haven't started working on this bug, feel free to take it.

Comment 3

[Anuj Agarwal [:anujagarwal464]]

Attachment: bug897359.diff

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

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?

Comment 6

[Anuj Agarwal [:anujagarwal464]]

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?

Comment 7

[Boris Zbarsky [:bzbarsky]]

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.

Comment 8

[Anuj Agarwal [:anujagarwal464]]

Attachment: bug897359.diff incomplete

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

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8408399 [details] [diff] [review]
bug897359.diff incomplete

That's odd, since you're removing nsCrypto::PopChallengeResponse, no?

Comment 10

[Boris Zbarsky [:bzbarsky]]

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.

Comment 11

[Anuj Agarwal [:anujagarwal464]]

Attachment: bug897359.diff -final

Try build was successful.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8408770 [details] [diff] [review]
bug897359.diff -final

r=me

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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