[Bug 195363] Review Request: esc and esc-xulrunner-devel

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: esc and esc-xulrunner-devel


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=195363





------- Additional Comments From jmagne at redhat.com  2006-07-25 18:40 EST -------
Thanks again for the great suggestions. I have completed as many as I could in
the short time.

My comments mixed with yours.

Some packaging issues:
- in general, packages almost always follow %{macro_name} format instead of
%macro_name
- you need to call gtk-update-icon-cache in %post for your icon to be visible in
the menus, etc.
- the vendor thing is a bit odd.  Why do you rename the icon to have the vendor
prefix?  I think I would probably just drop the vendor stuff altogether and pass
--vendor esc to desktop-file-install.

All addressed.


- why do you install the LICENSE into libdir?
Not changed.


- in general, if we put things in the menu in the default install we give them a
generic name (e.g, "Text Editor" instead of "gEdit").  Maybe "Smart Card Manager" ?

Done.


- you've got it in the wrong spot I think.  It probably makes more sense to be
in "Administration".

Done.

- It needs a root password, yes?  So you'll need to use consolehelper.  install
esc in /usr/sbin, create a symlink from /usr/bin/consolehelper to /usr/bin/esc
and install a file called esc to /etc/pam.d with this in it:

The app runs just fine as a regular user.


- If you want to start the monitoring bits at login, you'll need to install a
desktop file (like the one you put in /usr/share/applications) into
/etc/xdg/autostart .  Note, the program will be run as a normal user, not as
root, so you'll need to separate the management bits from the monitoring bits
for it to work.

Done.


- If you do start it at login, make sure you hide the icon until someone inserts
a security token.

This one I will have to figure out.


cosmetic issues:
1) the icon in the notification area is different than the icon in the app.
2) the gradient is a bit ugly
3) the spacing in the side frame is weird
4) some of the text is wonky, could probably use some proof reading

Work on the UI in general is ongoing including the above.


Some other things:

- When it starts up it asks for a config uri.  I gave the cseng one and it
didn't work.  It just gave me error code 28.  At some point I switched to
connecting to the mountain view vpn (from the westford one) and then it started
working.  I don't know if changing vpn's is what fixed it or if it only works
sometimes.

The current latest esc app is designed to be able to call back to the server to
get many quantities such as the TPS URL. Your server does not have this
functionality as of yet.
As a backup it still supports the "esc.tps.url" pref value in
/usr/lib/esc-1.0.0/defaults/preferences/esc-prefs.js. This can be set manually.



- If I click the test button on the config dialog it gives me another dialog
telling me it's about to do the test I just asked for.  that dialog isn't really
a good idea.

Done.


- It probably would be a good idea to disallow the token that was used for
logging in from being able to be formatted. 

Good idea.


- why is the log file for esc in libdir?

Now the file goes under the user's profile. Which on Linux is under:

~/.redhat/esc


Note, I never actually got enrollment to work.  It formatted my token fine, but
after a few blinks during the enrolment step the token led just turns off and
the client sits with two spinning throbbers indefinitely.  It's just sitting in
poll waiting for events I think.

This was a simple Javascript glitch which has been addressed.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.