In reviewing/updating the support docs for 1.0, I came across the above reference to this. smfr explained what it was on irc and we noted that despite the claims of the above-mentioned text that this was slated for post-1.0, there was no bug on this. [12:32am] smfr: currently we'll automatically use the first personal cert [12:32am] ardissone: so that text is mostly correct? [12:33am] smfr: well, we don't offer you an interface to let you choose which certificate to use [12:33am] smfr: but you will be able to log in if you have only one, or it chooses the appropriate one [12:33am] ardissone: ok [12:33am] ardissone: (is there a bug for fixing that? I didn't see anything that sounds like that) [12:35am] smfr: no

Actually the UI is there, but the pref to make it show up is not. To get the cert picker, set "security.default_personal_cert" to "Ask Every Time" (yeah, gotta love those human readable pref values).

So if I understand this correctly, I should modify the documentation text to add something like "if you have more than one personal certificate, set the pref to 'Ask Every Time' in order to be asked which certificate to use"? (The revised text as it stands is "Unfortunately, Camino does not offer an interface for choosing which personal certificate to use when accessing a website that requires a personal certificate. Camino always uses the first personal certificate it finds.") And this bug is now more about adding a prefs UI to enable the user to activate the security UI--or will simply having the options fully documented in the cb.o docs be enough?

This bug is really about adding radio buttons to the Security prefs panel to say: When a web site requires a certificate: O Select one automatically o Ask me every time If you want to mention hidden prefs on the documentation page, then by all means do so (or link to a "tips and tricks" page).

/me loves prefpanes

This: -Adds the radio matrix for this bug -Fixes bug 324608 -Fixes the second issue in bug 325880 comment 0 Also, bug 324608 comment 1 is something we should think about some. There's a potential problem here, which is that I couldn't trigger the dialog asking me to select a certificate. It certainly sets the pref correctly (verified with about:config)... Is the underlying support for this broken, or was my browser just on crack?

Does comment 3

Comment on attachment 227602 [details] [diff] [review] Patch + if(gotPref){ + if([certificateBehavior isEqual:@"Select Automatically"]) + selectRow = 0; + else if([certificateBehavior isEqual:@"Ask Every Time"]) + selectRow = 1; + } + [mCertificateBehavior selectCellAtRow:selectRow column:0]; Watch your spacing here, there should be once space like this: if (gotPref) { ... Perhaps you could set the row values as constants and then reference them in your if statement: const unsigned int kAskEveryTimeMatrixRowValue = 0; const unsigned int kSelectAutomaticallyMatrixRowValue = 1; ... [mCertificateBehavior selectCellAtRow:kAskEveryTimeRowValue column:0]; [mCertificateBehavior selectCellAtRow: kSelectAutomaticallyMatrixRowValue column:0]; +-(IBAction)clickCertificateSelectionBehavior:(id)sender +{ + unsigned int row = [mCertificateBehavior selectedRow]; + + if(row == 0) + [self setPref:"security.default_personal_cert" toString:@"Select Automatically"]; + else + [self setPref:"security.default_personal_cert" toString:@"Ask Every Time"]; +} Setting the constants as mentioned above would work great here. Lets give this another spin.

Comment on attachment 227610 [details] [diff] [review] Adresses kreeger's Comments Grr. My connection/bugzilla hicupped. Sorry for the bugspam

Comment on attachment 227612 [details] [diff] [review] Adresses kreeger's Comments +const unsigned int kSelectAutomaticallyMatrixRowValue = 0; +const unsigned int kAskEveryTimeMatrixRowValue = 1; Some spacing issues here, the committer can take care of these. + if ([certificateBehavior isEqual:@"Select Automatically"]) + [mCertificateBehavior selectCellAtRow:kSelectAutomaticallyMatrixRowValue column:0]; + else if ([certificateBehavior isEqual:@"Ask Every Time"]) + [mCertificateBehavior selectCellAtRow:kAskEveryTimeMatrixRowValue column:0]; + if (row == kSelectAutomaticallyMatrixRowValue) + [self setPref:"security.default_personal_cert" toString:@"Select Automatically"]; + else // row == kAskEveryTimeMatrixRowValue + [self setPref:"security.default_personal_cert" toString:@"Ask Every Time"]; A space between the if and the else would be nice if the comitter wants to. r=me

> There's a potential problem here, which is that I couldn't trigger the dialog > asking me to select a certificate. It certainly sets the pref correctly > (verified with about:config)... Is the underlying support for this broken, or > was my browser just on crack? I believe I was able to get this to happen many moons ago. It should hit SecurityDialogs::ChooseCertificate() which fires up a ChooseCertDialogController.

I suppose to test that, one needs 1) a site that requires a personal cert and 2) at least one, possibly more, personal certs. I did some searching on *.mozilla.org (thinking there might be a testcase somewhere, perhaps for Litmus) without any luck.

Since they're no longer supported. This just removes code, it doesn't add any, so still going w/ the kreeger r+.

Other than that, it's the same, so not bothering with a png. Bug me if you want one.

Unfortunately there are some discrepancies between 10.3 IB and 10.4 IB. Hopefully this will satisfy both.

Comment on attachment 227800 [details] Better Text for the new pref (nib) Sorry about all the nibspam; IB has lots of issues, and nib spacing seems to vary wildly between OS versions. Simon, since you actually understand what all these do, can you make sure the new nib options and hint text say what they need to? There wasn't room in the nib to do comment 3 verbatim.... "Final" screenshot to follow.

I think it needs to be laid out like: Certificates: When a site requires a certificate: O Select one automatically O Ask me which to use ( Certificates )

This does essentially what smfr asks, but adds "personal." I feel this is important to include somehow, since otherwise people who never get asked will either think that a) certificates aren't being used at all or b) the pref is broken. Even with that though, the double-colon thing (cert: enable cert when:) here is kind of ugly. I agree that there's no easy fix, but I personally prefer the previous incarnation to this one.

Putting on the late-night IRC polish. :)

Comment on attachment 227869 [details] Same design as "Adresses comment 23" Yay for 10.3 vs. 10.4 display/layout differences. Simon, this OK with you? (FWIW, I tend to agree with Ian that the previous incarnation looks better, and indenting the matrix on this incarnation makes it look even more odd.)

This one is easier to parse for the reader, I think, even if it may look a little odd. It's fine by me.

So all in all, this bug: - Exposes the security.default_personal_cert pref - Removes "Ask Permission When" title, since we're really just showing a warning - Removes the UI for weak ciphers warning, since it's deprecated - Plays the overall prefpane polish game

Comment on attachment 227682 [details] [diff] [review] Gets rid of weak ciphers sr=pink

Checked in on the trunk and MOZILLA_1_8_BRANCH.