Closed
Bug 750421
Opened 12 years ago
Closed 12 years ago
Remove unnecessary nsIBadCertListener2 and nsISSLErrorListener implementations
Categories
(Toolkit :: General, enhancement)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: briansmith, Assigned: briansmith)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
After bug 682329, there is no default UI for cert errors or SSL errors, so all the nsIBadCertListener2 and nsISSLErrorListener implementations that just "return true" can be removed.
Assignee | ||
Updated•12 years ago
|
Component: about:memory → General
QA Contact: about.memory → general
Blocks: 844351
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Comment 3•12 years ago
|
||
Brian, other than a little bitrot, what's preventing WIP #2 from moving forward? (btw, WIP #1 looks empty...)
Flags: needinfo?(bsmith)
Assignee | ||
Comment 5•12 years ago
|
||
IIRC, some part of the patch causes a test to fail, but I cannot remember which one.
Mostly, it is just a matter of me finding time to clean up the patch. If you want to push this forward, go ahead.
Flags: needinfo?(bsmith)
Ok - I did some cleanup and pushed it to try: https://tbpl.mozilla.org/?tree=Try&rev=6b764d9a2518
Well, that try run looked good, so I think this is ready for review.
Attachment #717413 -
Attachment is obsolete: true
Attachment #717414 -
Attachment is obsolete: true
Comment on attachment 719560 [details] [diff] [review]
patch
mayhemer: please review changes in netwerk/
Attachment #719560 -
Flags: review?(honzab.moz)
Comment on attachment 719560 [details] [diff] [review]
patch
dholbert: if you could review changes in content/, that's be great (this isn't urgent, but I'd like to avoid bitrot :)
Attachment #719560 -
Flags: review?(dholbert)
Comment on attachment 719560 [details] [diff] [review]
patch
dolske: if you could review changes in toolkit/, that's be great too (again, not super urgent)
Attachment #719560 -
Flags: review?(dolske)
Comment 11•12 years ago
|
||
(In reply to David Keeler (:keeler) from comment #10)
> dolske: if you could review changes in toolkit/, that's be great too (again,
> not super urgent)
I'm not really a reviewer for /content/base -- maybe ping a DOM peer, from https://wiki.mozilla.org/Modules/Core#Document_Object_Model ? (maybe Mounir?)
Comment on attachment 719560 [details] [diff] [review]
patch
Over to Mounir.
Attachment #719560 -
Flags: review?(dholbert) → review?(mounir)
Comment 13•12 years ago
|
||
Comment on attachment 719560 [details] [diff] [review]
patch
Review of attachment 719560 [details] [diff] [review]:
-----------------------------------------------------------------
r=me for the changes in content/base/.
Do you have someone to review the changes in dom/network/src/TCPSocket.js ?
Attachment #719560 -
Flags: review?(mounir) → review+
I don't, actually - if you could, that would be great (and anything else you can review, really).
Blocks: 846581
Updated•12 years ago
|
Attachment #719560 -
Flags: review?(dolske) → review+
Comment 15•12 years ago
|
||
Comment on attachment 719560 [details] [diff] [review]
patch
Review of attachment 719560 [details] [diff] [review]:
-----------------------------------------------------------------
Remove also a comment reference:
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsISpeculativeConnect.idl#25
I presume bug 846581 is going to land simultaneously with this patch, so the following won't fail:
http://hg.mozilla.org/mozilla-central/annotate/0a91da5f5eab/netwerk/test/unit/test_spdy.js#l297
r=honzab for the networking parts
Attachment #719560 -
Flags: review?(honzab.moz) → review+
This doesn't remove the nsIBadCertListener2 interface itself, so things like test_spdy.js#l297 should still work.
Thanks for the reviews, everyone, by the way.
Comment 18•12 years ago
|
||
(In reply to David Keeler (:keeler) from comment #14)
> I don't, actually - if you could, that would be great (and anything else you
> can review, really).
I wouldn't be comfortable reviewing that. I think you should ask the original author of the code (the chunk in dom/network/) to review your change.
Actually, I don't think the changes in dom/network should be in this patch. That nsIBadCertListener2 implementation does more than the others that are being removed and should be handled separately. This patch is the same as the previous, but without those changes and some unnecessary comment removals. Carrying over r+s. Here's another try run, for good measure: https://tbpl.mozilla.org/?tree=Try&rev=fce7c1137f04
Attachment #719560 -
Attachment is obsolete: true
Attachment #721821 -
Flags: review+
Comment 21•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment 22•12 years ago
|
||
Followups to unbreak Thunderbird and Seamonkey.
https://hg.mozilla.org/comm-central/rev/5110d56fbd96
https://hg.mozilla.org/comm-central/rev/8fb887a279ff
You need to log in
before you can comment on or make changes to this bug.
Description
•