Last Comment Bug 308062 - Trust CA dialogue: "trust this certif for" options text is cut off
: Trust CA dialogue: "trust this certif for" options text is cut off
Status: RESOLVED FIXED
[good first bug]
: fixed1.8.1, polish
Product: Camino Graveyard
Classification: Graveyard
Component: General (show other bugs)
: 1.8 Branch
: PowerPC Mac OS X
-- minor (vote)
: Camino1.5
Assigned To: Sean Murphy
:
:
Mentors:
http://www.black-helicopter.org/bh/ge...
Depends on:
Blocks: 325880
  Show dependency treegraph
 
Reported: 2005-09-11 14:51 PDT by Smokey Ardisson (offline for a while; not following bugs - do not email)
Modified: 2006-09-21 22:37 PDT (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
illustration of the issue (46.38 KB, image/jpeg)
2005-09-11 14:55 PDT, Smokey Ardisson (offline for a while; not following bugs - do not email)
no flags Details
Trust CA Dialog Patch (1.55 KB, patch)
2006-09-17 20:42 PDT, Sean Murphy
froodian: review+
sfraser_bugs: superreview+
Details | Diff | Splinter Review
DownloadCACertDialog.nib with increased window/sheet width (7.25 KB, application/zip)
2006-09-19 10:41 PDT, Sean Murphy
froodian: review+
Details
[Screenshot] Checkbox labels before/after (51.28 KB, image/gif)
2006-09-19 10:43 PDT, Sean Murphy
no flags Details
[Screenshot] RSA Key text fields before/after (51.46 KB, image/gif)
2006-09-19 10:44 PDT, Sean Murphy
no flags Details
[Screenshot] RSA Key text fields before/after (fixed alignment of keys) (57.61 KB, image/gif)
2006-09-19 10:51 PDT, Sean Murphy
no flags Details
[Screenshot] Shows what happens to the sheet title's descriptive text under French locale. (46.11 KB, image/png)
2006-09-19 10:55 PDT, Sean Murphy
no flags Details

Description User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-09-11 14:51:21 PDT
In the new Trust a new CA dialogue, the text of the "Trust this certificate
for:" options is cut off by default on the right.  The dialogue should be wide
enough by default.
Comment 1 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-09-11 14:55:00 PDT
Created attachment 195677 [details]
illustration of the issue
Comment 2 User image Jasper 2005-09-11 15:13:27 PDT
I'd say it's save and UI&layout wise good to just shift that whole block of
checkboxes to the left so that the gap is filled.
Comment 3 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-09-11 15:27:04 PDT
I tend to agree with Jasper; that block looks odd sitting out there, since
there's not another block in the left-hand column there (but it is lined up with
all the other right-hand column blocks).

The same thing appears in the certificate viewing window (where it's not cut
off), so if we move the entire block, we should also do so in certificate
viewing window, too.
Comment 4 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-12-04 02:02:24 PST
Simon, is this easy to fix or should we push this off?
Comment 5 User image Simon Fraser 2005-12-04 10:28:32 PST
Should just be a nib window resize.
Comment 6 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-12-11 01:39:35 PST
I imagine this is a problem in all of the certificate sheets.  I tried to fix it, but I somehow broke the nib trying to resize the CertificateView to match the resized scrollbox; I need to enroll in ".nib editing 102" before I can fix this one myself.
Comment 7 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-01-20 01:19:41 PST
smfr, will you be able to make these changes for 1.0, if it's just resizing the nib windows?
Comment 8 User image Samuel Sidler (old account; do not CC) 2006-02-03 15:19:33 PST
-> 1.1
Comment 9 User image Sean Murphy 2006-09-17 20:42:54 PDT
Created attachment 238961 [details] [diff] [review]
Trust CA Dialog Patch

To keep the text from cutting off, we could have changed the starting frame size for the sheet which is displayed, but rather than take that approach, I modified the CertificateView code which programmatically inserts the checkbox views into the dialog.

The checkbox group now aligns to the same column as the top header labels.  Its x position is determined in the exact same way the header text fields calculate theirs.  So, the look and feel of the sheet is still balanced and appears nice and polished, despite moving around the checkboxes.  (Also looks fine with the extra details expanded.)

The only hitch is that there still might not be enough room for the text of other localizations.  We may have to check this..
Comment 10 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-09-17 20:59:51 PDT
Reassigning this to Sean :)
Comment 11 User image Nick Kreeger 2006-09-17 22:34:28 PDT
(In reply to comment #9)
> Created an attachment (id=238961) [edit]
> Trust CA Dialog Patch
> 
> To keep the text from cutting off, we could have changed the starting frame
> size for the sheet which is displayed, but rather than take that approach, I
> modified the CertificateView code which programmatically inserts the checkbox
> views into the dialog.
> 
> The checkbox group now aligns to the same column as the top header labels.  Its
> x position is determined in the exact same way the header text fields calculate
> theirs.  So, the look and feel of the sheet is still balanced and appears nice
> and polished, despite moving around the checkboxes.  (Also looks fine with the
> extra details expanded.)
> 
> The only hitch is that there still might not be enough room for the text of
> other localizations.  We may have to check this..
> 

Correct, this approach only fixes this problem for some localizations. We should really find a way of telling the UI to wrap the text, or do the wrap drawing our selves.

Comment 12 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-09-17 22:54:53 PDT
Comment on attachment 238961 [details] [diff] [review]
Trust CA Dialog Patch

Yeah, that sheet is pretty badly broken in other languages as-is.  (The more common one, the url mismatch sheet, is in better shape).  But let's make reworking the sheet another bug for later.  This patch is a significant improvement for everyone.
Comment 13 User image froodian (Ian Leue) 2006-09-17 23:02:47 PDT
Comment on attachment 238961 [details] [diff] [review]
Trust CA Dialog Patch

I agree that this is a major improvement (even aesthetically IMO - the checkboxes look nice lined up with the header fields), so r=me on the patch.

Just to give it a little more elbow room, could you submit a revised nib (just zip it up and attach the whole thing) with a slightly wider window (and possibly slightly wider minimum width as well)?

r=me on the code changes, and a=me for the fix with a nib.
Comment 14 User image Sean Murphy 2006-09-19 10:41:19 PDT
Created attachment 239194 [details]
DownloadCACertDialog.nib with increased window/sheet width

The initial width of the sheet has been increased from 550 to 600, which, when combined with the previous patch, prevents any of the labels from being cut short.  This nib resize also enables the RSA key text fields (when details are expanded) to fit cleanly without extra line breaks.  The minimum allowed width has also been changed to be identical to this new initial width, since any smaller size causes issues.

An additional problem with this nib is that the mMessageField IBOutlet (which is the descriptive text under the sheet heading saying: "You have obtained a certificate...") has its string value set to a different value programmatically.  This means that to the localizing team, the label will appear to fit all text correctly, but the value which is set in code turns out to be much longer (as happens in the French version, for instance).  If this dialog (DownloadCACertDialog) always displays the same description text, should we just set this in Interface Builder and just use a format token for any unique info (such as cert name)?   

Also, speaking of localizations, ones that require longer labels will still have the original bug (cut-off checkbox labels) occur.  I don't mind reworking the way the sheet is presented, (so this doesn't continue to cause trouble) but should this be done under a different bug report?  Or, are there other issues which deserve attention first, leaving this for later (since this fix does work in most cases)?

[See attached before/after images]

I'll need another review on this, plus a super, I believe.. Thanks everyone for all the comments so far.

Note: This was my first patch submitted to the Camino project and I look forward to becoming involved, especially fixing some more bugs.  This is awesome work and I'm glad to help out a small part of it..
Comment 15 User image Sean Murphy 2006-09-19 10:43:44 PDT
Created attachment 239195 [details]
[Screenshot] Checkbox labels before/after
Comment 16 User image Sean Murphy 2006-09-19 10:44:21 PDT
Created attachment 239196 [details]
[Screenshot] RSA Key text fields before/after
Comment 17 User image Sean Murphy 2006-09-19 10:51:28 PDT
Created attachment 239197 [details]
[Screenshot] RSA Key text fields before/after (fixed alignment of keys)
Comment 18 User image Sean Murphy 2006-09-19 10:55:02 PDT
Created attachment 239200 [details]
[Screenshot] Shows what happens to the sheet title's descriptive text under French locale.
Comment 19 User image Sean Murphy 2006-09-19 22:02:07 PDT
I don't want to get too off-topic from the original bug, but concerning mMessageField's stringValue, setting the text inside Interface Builder would require us to sacrifice using the more informative format string (which inserts the certificate name into the description at %@).  The only way, unless I'm forgetting something, to avoid a setStringValue message and still use a format string would be binding displayPatternValue, which isn't a solution for us.  So, on second thought, it might be best to leave the dialog alone.

If anything is changed, maybe just set the text field value in IB to the string that is actually inserted during runtime (when [mMessageField setStringValue:] is called).  I'm thinking those who localized this nib didn't realize the description string would be replaced with a longer one by code, and were sure it just looked alright.

I hope this makes sense.  I should mention again that the original bug is fixed with the above patch and nib (with some locales excluded, which we've acknowledged).
Comment 20 User image froodian (Ian Leue) 2006-09-20 22:34:13 PDT
Comment on attachment 238961 [details] [diff] [review]
Trust CA Dialog Patch

Simon, do you think you could look at this for sr?  It still leaves some to be wanted for localizations, but it's worlds better than what we have now, and we can file a followup on making it "localization-proof" ;)
Comment 21 User image froodian (Ian Leue) 2006-09-21 22:37:43 PDT
Checked in on 1.8branch and trunk.

Note You need to log in before you can comment on or make changes to this bug.