Closed
Bug 575950
Opened 14 years ago
Closed 14 years ago
SSL certificate is not confirmed for secure webpages.
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(fennec2.0b2+)
VERIFIED
FIXED
Firefox 4.0
Tracking | Status | |
---|---|---|
fennec | 2.0b2+ | --- |
People
(Reporter: vladmaniac, Assigned: mayhemer)
References
(Blocks 1 open bug, )
Details
Attachments
(3 files, 10 obsolete files)
89.26 KB,
image/png
|
Details | |
7.70 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
73.51 KB,
patch
|
KaiE
:
review+
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:2.0b2pre) Gecko/20100629 Minefield/4.0b2pre Build Identifier: 20100629041239 No green or blue "Larry" icon is shown. Both for the untrusted connections and secure ones (with SSL cert) a grey "Larry" is displayed. Reproducible: Always Steps to Reproduce: 1.Navigate https://www.paypal.com 2.Click on the site's favicon 3.Inspect the applet being display correctly with no errors of any kind. Actual Results: Green security guard should appear. Expected Results: Grey security icon is displayed instead, specifically for untrusted sites, as here is not the case.
Reporter | ||
Comment 1•14 years ago
|
||
Comment 2•14 years ago
|
||
This works for me on build: Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b2pre) Gecko/2010630 Namoroka/4.0b2pre Fennec/2.0a1pre both on trunk and fennec-electrolysis-maemo5-gtk builds.
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
Comment 3•14 years ago
|
||
I still see this problem
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•14 years ago
|
||
It's still broken today, but it was working at changeset 39d29c21907c I'll bisect from here.
Reporter | ||
Comment 5•14 years ago
|
||
I can confirm that. I was just about to close the bug to resolved:worksforme
Updated•14 years ago
|
tracking-fennec: --- → 2.0b2+
Comment 6•14 years ago
|
||
Fabrice, can you take a look at this? We have access to a better secureBrowserUI now. See bug 536301 and bug 568502.
Updated•14 years ago
|
Assignee: nobody → fabrice
Comment 7•14 years ago
|
||
This log shows the activity of the nsSecureBrowserUIImpl whild loading a simple https page (https://demo.pairssl.com/) The log shows that in GetSecurityStateFromSecurityInfo() the info parameter is null. I think this comes from the call to ExtractSecurityInfo(nsIRequest) where we fail to get securityInfo from the request's channel or from the request's implementation of nsISecurityInfoProvider
Assignee | ||
Comment 8•14 years ago
|
||
Then it looks the patch from bug 536301 stopped working. It was exactly created to provide security info object on the content process (attach it to HttpChannelChild where from the sec UI impl is trying to extract it). I'll re-check this with multiprocess firefox build first.
Assignee | ||
Comment 9•14 years ago
|
||
This is really bad. We correctly send security info serialization to HttpChannelChild, but we fail to instantiate nsNSSSecurityInfo object because we do not allow nsNSSComponent (and the whole NSS) be instantiated on the child process on which nsNSSSecurityInfo depends. This is intentional by bug 559711. Sorry, I had to catch this much sooner, this has been broken since 2010-08-10. Options: 1. back bug 559711 out again (simple, but I cannot say what all this will break) 2. check if nsNSSSecurityInfo and other classes necessary to determine security state are really dependent on nsNSSComponent and bypass dep check on child process (I don't think we success in this, but I will try to) 3. do a major architectural change where we have a mirror of loadgroups/docshells on the parent process and track the state there as I proposed months ago (it's clear it's way too late for this option)
Blocks: 559711
Assignee | ||
Comment 10•14 years ago
|
||
- makes some PSM components instantiable on non-chrome processes - nsNSSCertificate only keeps the cert serialization on non-chrome processes ; instantiating mCert fails because it needs NSS be initialized
Updated•14 years ago
|
Status: ASSIGNED → NEW
tracking-fennec: 2.0b2+ → 2.0b1+
Comment 11•14 years ago
|
||
Honza, I did a build with your v1 patch, but was unable to load any SSL site with it applied.
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11) > Honza, I did a build with your v1 patch, but was unable to load any SSL site > with it applied. What exactly means 'unable to load' ?
Comment 13•14 years ago
|
||
(In reply to comment #12) > (In reply to comment #11) > > Honza, I did a build with your v1 patch, but was unable to load any SSL site > > with it applied. > > What exactly means 'unable to load' ? The loading throbber runs forever, with no content being displayed.
Assignee | ||
Comment 14•14 years ago
|
||
Fabrice, would be good to check if this is not a dup of bug 592197, that already has a patch.
Comment 15•14 years ago
|
||
I tried several builds : - patch in bug 592197 only : page loads, but doesn't display SSL Status - patch in bug 592197 and Honza's : page does not load (we've seen in logs that we are actually doing two request, the first one being immediately canceled and the second one is stuck : the child process doesn't even get OnStartRequest)
Comment 16•14 years ago
|
||
What's the best way to proceed here? We can start snooping with gdb, but it would be nice to have some expectations for how it should be working. Fabrice - I guess you could test the code paths in Firefox (or Fennec with browser.tabs.remote=false) and see how it is supposed to work. Then compare that to what you see when remote=true
Comment 17•14 years ago
|
||
Can we just deserialize the security info into a child-side-only impl?
Assignee | ||
Comment 18•14 years ago
|
||
(In reply to comment #17) > Can we just deserialize the security info into a child-side-only impl? Boris, sorry, I don't follow what you mean. The security info is sent as a string-serialization to the child process through Send/RecvOnStartRequest. The problem I was solving with my patch is that we cannot de/serialize certificates w/o having the nss initialized on the child process (bug 559711).
Comment 19•14 years ago
|
||
I don't understand why the content process needs to know anything about it, other than perhaps storing a serialized certificate or a certificate handle and sending it back to the chrome process at some point. Although really, I'd prefer if the chrome process did all of the certificate tracking, but that probably requires mirrored load groups and such.
Assignee | ||
Comment 20•14 years ago
|
||
(In reply to comment #19) > ...requires mirrored load groups and such. Exactly.
Comment 21•14 years ago
|
||
> The problem I was solving with my patch is that we cannot de/serialize
> certificates w/o having the nss initialized on the child process
Why not?
Assignee | ||
Comment 22•14 years ago
|
||
(In reply to comment #21) > Why not? It's using NSS code to parse DER data and create nss representation of the certificate, that nsNSSCertificate component only wraps.
Comment 23•14 years ago
|
||
OK. I assume that skipping all that and just having a non-NSS impl of nsIX509Cert is too much pain?
Assignee | ||
Comment 24•14 years ago
|
||
So, what you suggest is not to modify the current nsNSSCertificate code, but create a new class, registered under the same CID/contract, that is used on content (or in general - non-NSS) processes, and can be used just for carrying the serialization?
Comment 25•14 years ago
|
||
I'm saying it's my gut instinct to try that, yes. I haven't thought about this enough to say whether I'd go so far as _suggesting_ it. ;)
Comment 26•14 years ago
|
||
Honza, how close are you with this? Fennec needs this.
Assignee | ||
Comment 27•14 years ago
|
||
If it is that urgent I can have a patch today probably.
Assignee | ||
Comment 28•14 years ago
|
||
OK, work in progress, but finished probably tomorrow morning. Is that OK?
Comment 29•14 years ago
|
||
(In reply to comment #28) > OK, work in progress, but finished probably tomorrow morning. Is that OK? Should be fine - Thanks
Assignee | ||
Comment 30•14 years ago
|
||
Needs someone to test this please! I don't have an environment to run my e10s patches at this time, and it doesn't look well to have one soon. So, if anyone can take this patch and test it (actually run it for the first time) it would be of great help. Thanks.
Comment 31•14 years ago
|
||
I'm doing a build right now.
Comment 32•14 years ago
|
||
Fabrice said the patch in this doesn't work -- Fabrice, do you have more info?
Updated•14 years ago
|
tracking-fennec: 2.0b1+ → 2.0b2+
Comment 33•14 years ago
|
||
Here's the log I got loading https://demo.pairssl.com/ with the log set as : NSPR_LOG_MODULES=timestamp,nsHttp:5,nsSocketTransport:5,nsSecureBrowserUI:5 SecurityUI info starts to appear at line 1053 The securityState appears to be 262146 (0x40002) which means STATE_IS_SECURE | STATE_SECURE_HIGH I'll check again what the frontend is doing with this.
Comment 34•14 years ago
|
||
I did more testing : - when tabs.remote == true, we get an assertion : ###!!! ASSERTION: Trying to initialize PSM/NSS in a non-chrome process!: 'Error', file /home/ifda7415/dev/mozilla-central/security/manager/ssl/src/nsNSSComponent.cpp, line 302 and subsequently JS errors : [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIX509Cert.organization]" nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)" location: "JS frame :: chrome://browser/content/bindings/browser.js :: anonymous :: line 137" data: no] - when tabs.remote == false, it works except if we load an SSL page as the first page of a session (we get the SSL status for the subsequent loads).
Assignee | ||
Comment 35•14 years ago
|
||
Comment on attachment 470184 [details] [diff] [review] v1 It was decided to use different classes for chrome and content process. This patch doesn't do that.
Attachment #470184 -
Attachment is obsolete: true
Attachment #470184 -
Flags: review?(kaie)
Assignee | ||
Comment 36•14 years ago
|
||
Comment on attachment 475178 [details] [diff] [review] v2 I have found out that on many places PSM calls new nsNSSCertificate. Although those places seems as well dependent on NSS, so probably should NOT get invoked on child process thanks prevention of NSS instantiation, I'll wrap all new nsNSSCertificate to static nsNSSCertificate::Create method, that will return null on child process. I already started to work on that patch.
Attachment #475178 -
Attachment is obsolete: true
Assignee | ||
Comment 37•14 years ago
|
||
Improved patch.
Assignee | ||
Comment 38•14 years ago
|
||
qref...
Assignee | ||
Updated•14 years ago
|
Attachment #476363 -
Attachment is obsolete: true
Comment 39•14 years ago
|
||
honza, any update here?
Updated•14 years ago
|
Severity: major → critical
Priority: -- → P1
Assignee | ||
Comment 40•14 years ago
|
||
Doug, the patch should fix the problem with serialization, but there seems to be some different problem in Fennec yet. I don't have a working desktop build of Fennec to test, it seems the desktop build is broken... Brian Crowder was ones helping me to find out what is wrong, but we've quit the work undone w/o a conclusion and we haven't meet again since than.
Assignee | ||
Comment 42•14 years ago
|
||
OK, I got my windows Fennec desktop build! credits: jdm, cjones. Almost there, but few flaws: 1. I test with bugzilla.mozilla.org page, on its first load I don't get the final STATE_STOP notification for the top level document, that would trigger the OnSecurityChange notification from nsSecureBrowserUIImpl ; I get it on reload. Not sure where it gets lost, but will investigate further. If anyone knows something more, please make a note. 2. We correctly get here [1] with the patch but SecurityUI.getIdentityData reads the certificate properties that are inaccessible on the content process with the patch (like subjectName etc. as depending on NSS init). I need help with the second issue as I don't know the code enough. The notification passes this way: 1. /chrome/content/bindings/browser.js, onSecurityChange, the nsIWebProgress imlementaion, called on content process 2. /chrome/content/bindings/browser.xml, remote-browser, receiveMessage("WebProgress:SecurityChange"), _called on CHROME process ???_ 3. /chrome/content/browser.js, onSecurityChange, the nsIWebProgress imlementaion, called on the chrome process I assume If the second step happens on the chrome process, we may send the serialization and not the properties. Then instantiate the whole security info, as the code removed in bug 582840 did. [1] http://hg.mozilla.org/mobile-browser/annotate/450b16d94ac9/chrome/content/bindings/browser.js#l76
Status: NEW → ASSIGNED
Comment 43•14 years ago
|
||
(In reply to comment #42) > 2. /chrome/content/bindings/browser.xml, remote-browser, > receiveMessage("WebProgress:SecurityChange"), _called on CHROME process ???_ Yes, browser.xml is in the chrome process.
Comment 44•14 years ago
|
||
> 1. I test with bugzilla.mozilla.org page, on its first load I don't get the
final STATE_STOP notification for the top level document, that would trigger
the OnSecurityChange notification from nsSecureBrowserUIImpl ; I get it on
reload.
The last time I saw something like that, it turned out to be the channel not removing itself from the loadGroup.
Assignee | ||
Comment 45•14 years ago
|
||
I need to carry the certificate serialization between processes. The code where it has to happen is, however, written in JS. I don't know about any helper usable in JS that would make it simple to de/serialize an object to a string. So, I plan on one of the following: 1. Create a new interface for nsNSSCertificate like nsINSSCertificateSerialization, that would access the cert serialization as a string, both classes (for chrome and content) would implement it. 2. Probably a better solution as we might want to carry also other objects in the future, create a component that could be used in JS to simply de/serialize objects, the same as functions from nsSerializationHelper.cpp do, but scriptable (a new bug for that).
Comment 46•14 years ago
|
||
(In reply to comment #45) > I need to carry the certificate serialization between processes. The code > where it has to happen is, however, written in JS. I don't know about any > helper usable in JS that would make it simple to de/serialize an object to a > string. Honza - explain a little more about what you need and where the code (JS) is that needs to use it. We have a few message managers that can de/serialize object across processes. We use them for helping in various JS XPCOM components. nsContentPrefService.js is one example: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/contentprefs/src/nsContentPrefService.js#50
Assignee | ||
Comment 47•14 years ago
|
||
On the content process I need to serialize the object through nsISerializable interface. For nsNSSCertificate it is the only interface that is implemented on the content process as any other interfaces' properties need NSS initialized to return meaningful results, we don't have NSS on the content process, right? Then the result binary data, probably best converted to a base64 string, carry from the content process to the chrome process. We already do this opposite way in C++ in PHttpChannel::OnStartRequest and we were doing this in PBrowser::NotifySecuryStateChange, that had been removed in bug 582840, and that I now need to re-create as part of this bug. The exact place where I need to serialize the certificate is: /chrome/content/bindings/browser.js, onSecurityChange (content process) The exact place where I need to deserialize the certificate is: /chrome/content/bindings/browser.xml, remote-browser, receiveMessage("WebProgress:SecurityChange"), (chrome process, as you confirmed) So, I just want to have a JS helper that makes it easier to convert an object implementing nsISerializable to a string and back.
Assignee | ||
Comment 48•14 years ago
|
||
I can't find anything in nsContentPrefService.js that I can use to do this.
Assignee | ||
Comment 49•14 years ago
|
||
Mozilla-central changes, wip patch.
Attachment #476366 -
Attachment is obsolete: true
Assignee | ||
Comment 50•14 years ago
|
||
Mobile changes. Few questions: 1. How does Fennec display Extended Validation state (the green SSL UI) ? I don't see it would be carried out in object returned from SecurityUI.getIdentityData(). 2. What is the substitute for content.location (present in browser.js on the content process) on the chrome process in browser.xml? IMPORTANT: When testing this patch, please remember it is not addressing the problem with missing STATE_STOP notification on the first page load, so you have to refresh the page to get the security UI. That is probably a different bug.
Attachment #469914 -
Attachment is obsolete: true
Attachment #475868 -
Attachment is obsolete: true
Comment 51•14 years ago
|
||
(In reply to comment #50) > 1. How does Fennec display Extended Validation state (the green SSL UI) ? I > don't see it would be carried out in object returned from > SecurityUI.getIdentityData(). Not exactly sure what you mean here, but Fennec also shows a green favicon for EV state. We use CSS on the "mode" attribute. See here: http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1506 setMode will set the "mode" attribute to "verifiedIdentity" and CSS does the rest. > 2. What is the substitute for content.location (present in browser.js on the > content process) on the chrome process in browser.xml? browser.currentURI.spec
Assignee | ||
Comment 52•14 years ago
|
||
Thanks for info. Where exactly in the current code is the EV state handled? EV status is not displayed with my patches, so it needs to be fixed as well.
Comment 53•14 years ago
|
||
(In reply to comment #52) > Thanks for info. Where exactly in the current code is the EV state handled? > EV status is not displayed with my patches, so it needs to be fixed as well. I don't know what you mean by "EV state handled" exactly so I am going to give you way to much information and hopefully something will answer your question. If a website has an EV cert the favicon turns green and you tap/click the favicon you'll see the site identification panel. The EV strings for the panel are loaded here: http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1615 The data used for the strings comes from <browser>.securityUI.SSLStatus.serverCert. That is originally set in content process here: http://mxr.mozilla.org/mobile-browser/source/chrome/content/bindings/browser.js#84 And passed to the chrome process and held here: http://mxr.mozilla.org/mobile-browser/source/chrome/content/bindings/browser.xml#56 One ugly aspect of this code (browser.xml#56) is that calling the getter actually starts the process of initializing the securityUI in the content process. The toolkit <browser> constructor seems to access the securityUI property and that makes it all work. Still kinda hacky IMO.
Assignee | ||
Comment 54•14 years ago
|
||
mark, thanks a lot, this is helpful. One more question: where is this code [1] executed? Chrome or content? [1] http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser.js#1491
Assignee | ||
Comment 55•14 years ago
|
||
This seems to be ready for review: - full SSL status object serialization is carried from content to chrome process - getIdentityData moved from content to chrome process, because on the chrome process there are no certificate properties to read, it caries only the serialization - ev status is specially handled on the chrome process because NSS assistance is needed
Attachment #478540 -
Attachment is obsolete: true
Attachment #478579 -
Flags: review?(mark.finkle)
Comment 56•14 years ago
|
||
Comment on attachment 478579 [details] [diff] [review] v3.1 (mobile) >+ let serialization = SecurityUI.getSSLStatusSerialization(); getSSLStatusSerialization -> getSSLStatus "Serialization" is a bit too long >diff --git a/chrome/content/bindings/browser.xml b/chrome/content/bindings/browser.xml >+ this._browser._securityUI = { >+ SSLStatus: SSLStatus ? {state: json.state, serverCert: data} : null, >+ state: json.state SSLStatus: SSLStatus ? { serverCert: data } : null, "state" does not need to be SSLStatus I can make any changes on checkin, if needed
Attachment #478579 -
Flags: review?(mark.finkle) → review+
Comment 57•14 years ago
|
||
With these two patches and the patch in bug 599632, SSL "larry" works in Fennec
Assignee | ||
Comment 58•14 years ago
|
||
(In reply to comment #56) > >+ let serialization = SecurityUI.getSSLStatusSerialization(); > > getSSLStatusSerialization -> getSSLStatus I don't agree here. SSLStatus means the object. But what the method returns is the object's serialization. I want to avoid any confusion here, so I'd rather leave the 'Serialization' here. Or, would you be more OK with 'AsString' instead? > "state" does not need to be SSLStatus True, overlook. Will update the patch.
Assignee | ||
Comment 59•14 years ago
|
||
Comment on attachment 478539 [details] [diff] [review] v3 (m-c) It turns out this is a full working version. Kai: please review the security/ changes: - created a new class nsNSSCertificateChild that is instantiated under the same CID and ContractID on the _child_ process instead of nsNSSCertificate - some changes to nsNSSModule.cpp that helps to handle it Boris: please review the netwerk/ changes: - just added an interface and component that does the same as serialization helper functions do but scripts can use it To both: this patch is very simple although it looks large.
Attachment #478539 -
Attachment description: v3 wip → v3 (m-c)
Attachment #478539 -
Flags: review?(kaie)
Attachment #478539 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 60•14 years ago
|
||
Serialization changed to AsString.
Attachment #478579 -
Attachment is obsolete: true
Attachment #482279 -
Flags: review+
Comment 61•14 years ago
|
||
Comment on attachment 482279 [details] [diff] [review] v3.2 (mobile) [Check in comment 82] AsString is fine by me
Comment 62•14 years ago
|
||
Comment on attachment 478539 [details] [diff] [review] v3 (m-c) This new component should be in XPCOM, not necko, no?
Comment 64•14 years ago
|
||
Comment on attachment 478539 [details] [diff] [review] v3 (m-c) (a) > nsNSSCertificate* > nsNSSCertificate::ConstructFromDER(char *certDER, int derLen) > { >- nsNSSCertificate* newObject = new nsNSSCertificate(); >+#ifdef MOZ_IPC >+ // On non-chrome process prevent instantiation >+ if (GeckoProcessType_Default != XRE_GetProcessType()) >+ return nsnull; >+#endif >+ >+ nsNSSCertificate* newObject = nsNSSCertificate::Create(); While you're touching this, please add some kind of "newObject not null" check, before the following "->" > if (!newObject->InitFromDER(certDER, derLen)) { > delete newObject; > newObject = nsnull; > } (b) > nsNSSCertificate::nsNSSCertificate(CERTCertificate *cert) : > mCert(nsnull), > mPermDelete(PR_FALSE), > mCertType(CERT_TYPE_NOT_YET_INITIALIZED), > mCachedEVStatus(ev_status_unknown) > { >+#ifdef MOZ_IPC >+ if (GeckoProcessType_Default != XRE_GetProcessType()) >+ NS_ERROR("Trying to initialize nsNSSCertificate in a non-chrome process!"); >+#endif NS_ERROR is a DebugBreak, only in debug builds, no action in optimized builds. Is this what you want? If yes, let's avoid to evaluate the expression in optimized builds, maybe by wrapping in #ifdef DEBUG ? (similar code more than once) (c) + mCertSerialization = SECITEM_AllocItem(nsnull, nsnull, len); + if (!mCertSerialization) + return NS_ERROR_OUT_OF_MEMORY; + PORT_Memcpy(mCertSerialization->data, const_cast<char*>(str.get()), len); I'm surprised you need a const_cast. I think str.get() should return (const char*) and second param to memcpy is a const char*. What compiler error do you get if you remove the cast? (d) You have a comment that says: + // On a non-chrome process we don't have mCert because we lack + // nsNSSComponent. nsNSSCertificateChild object is used only to carry the + // certificate serialization. Let's use a more descriptive name than nsNSSCertificateChild. I propose something like nsNSSCertFakeTransporter (e) + // If the component needs PSM/NSS initialized only on the chrome process, + // pretend we successfully initiated it but in reality we bypass it. + // It's up to the programmer to check for process type in such components + // and take care not to call anything that needs NSS/PSM initiated. scary! Do you assume the current code will not call any of the non-implemented functions? Did you try to replace return NS_ERROR_NOT_IMPLEMENTED; with PR_Assert(0); to see if you get an abort? (f) The changes to the factory constructors looked scary, too, on first sight, but I couldn't spot any obvious mistakes. (g) What do you think about bz's proposal to move the netwerk/ code to xpcom/ ?
Attachment #478539 -
Flags: review?(kaie) → review-
Assignee | ||
Comment 66•14 years ago
|
||
(In reply to comment #62) > Comment on attachment 478539 [details] [diff] [review] > v3 (m-c) > > This new component should be in XPCOM, not necko, no? It's using NS_SerializeToString and NS_DeserializeObject that are both declared/defined in netwerk. So, I'd argue to leave it in netwerk.
Comment 67•14 years ago
|
||
> It's using NS_SerializeToString and NS_DeserializeObject that are both
> declared/defined in netwerk.
Hmph. Those should live in xpcom too..
Maybe file a followup on moving that all there?
Assignee | ||
Comment 68•14 years ago
|
||
(In reply to comment #67) > Maybe file a followup Bug 605995.
Assignee | ||
Comment 69•14 years ago
|
||
(In reply to comment #64) > (e) > scary! > > Do you assume the current code will not call any of the non-implemented > functions? > > Did you try to replace > return NS_ERROR_NOT_IMPLEMENTED; > with > PR_Assert(0); > to see if you get an abort? > I went through mxr checking all properties that are unimplemented by this new class. No-one is accessing them on the content process.
Assignee | ||
Comment 70•14 years ago
|
||
(In reply to comment #68) > (In reply to comment #67) > > Maybe file a followup > Bug 605995. Boris, I'll move the component in that bug. Otherwise the code is OK? I'll soon update the patch with Kai's comments.
Assignee | ||
Comment 71•14 years ago
|
||
kaie: a,b,c,d addressed. e explained in comment 69 boris: please review only the network changes
Attachment #478539 -
Attachment is obsolete: true
Attachment #485144 -
Flags: review?(kaie)
Attachment #485144 -
Flags: review?(bzbarsky)
Attachment #478539 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 72•14 years ago
|
||
Comment 73•14 years ago
|
||
> Otherwise the code is OK?
I need to look again; I forget whether I read the rest of it the first time...
Comment 75•14 years ago
|
||
Comment on attachment 485144 [details] [diff] [review] v3.1 (mozilla-central) [Check in comment 80] Thanks, r=kaie
Attachment #485144 -
Flags: review?(kaie) → review+
Comment 76•14 years ago
|
||
(In reply to comment #75) > > Thanks, r=kaie (r= for the changes in security, I didn't look at netwerk)
Testers note: please verify theme change : Bug 575403 when regressing this bug in locked, unlocked, invalid cert, etc.
Comment 78•14 years ago
|
||
Comment on attachment 485144 [details] [diff] [review] v3.1 (mozilla-central) [Check in comment 80] r=me, though this new service seems to be unused and untested... I assume there are plans to change both?
Attachment #485144 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 79•14 years ago
|
||
(In reply to comment #78) > Comment on attachment 485144 [details] [diff] [review] > v3.1 (mozilla-central) > > r=me, though this new service seems to be unused and untested... I assume > there are plans to change both? It is used in the mobile patch.
Assignee | ||
Updated•14 years ago
|
Attachment #485145 -
Attachment is obsolete: true
Assignee | ||
Comment 80•14 years ago
|
||
Comment on attachment 485144 [details] [diff] [review] v3.1 (mozilla-central) [Check in comment 80] http://hg.mozilla.org/mozilla-central/rev/45376975eca8
Attachment #485144 -
Attachment description: v3.1 (mozilla-central) → v3.1 (mozilla-central) [Check in comment 80]
Assignee | ||
Comment 81•14 years ago
|
||
And a bustage fix for gcc builds: http://hg.mozilla.org/mozilla-central/rev/116ab7c785e1
Assignee | ||
Comment 82•14 years ago
|
||
Comment on attachment 482279 [details] [diff] [review] v3.2 (mobile) [Check in comment 82] http://hg.mozilla.org/mobile-browser/rev/c2cfa42b9157
Attachment #482279 -
Attachment description: v3.2 (mobile) → v3.2 (mobile) [Check in comment 82]
Assignee | ||
Comment 83•14 years ago
|
||
Patches landed on m-c and mobile. Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
Target Milestone: --- → fennec2.0
Green cert can be seen by going to something like paypal.com Blue cert can be seen by going to something like gmail.com Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2pre Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•