Re: [rfc][icedtea-web] Temporary files location chooser cleanup
Jacob Wisor
gitne at excite.co.jp
Tue Sep 3 07:38:50 PDT 2013
"Jiri Vanek"<jvanek at xxxxxxxxxx> wrote:
> On 08/01/2013 05:14 PM, Jacob Wisor wrote:
> > Hello,
> >
> > * Made temporary files location JFileChooser open at the currently specified location
> > * Made temporary files location JFileChooser display a helpful title
> > * Removed misleading "All Files" file filter from JFileChooser
> > * Added new message to resources for JFileChooser's choose button
> > * Fixed a few inconsistent messages in resource files
>
> Thank you, such an "make it looks better" contributions are more then welcomed!
> >
> > @Jiri
> > Please add a cs translation for the new "TIFPFileChooserChooseButton" message.
>
> It will be "Vybrat", but I prefer to sync the translations in longer time periods in bulk.
I understand. Please see further down.
> >
> >
> > Temporary files FileChooser cleanup.patch
> >
> >
> > diff -r f1eaa1ee7891 netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java
> > --- a/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java
> > +++ b/netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java
> > @@ -109,10 +109,12 @@
> > bLocation.addActionListener(new ActionListener() {
> > @Override
> > public void actionPerformed(ActionEvent e) {
> > - JFileChooser fileChooser = new JFileChooser();
> > + JFileChooser fileChooser = new JFileChooser(location.getText());
>
> Tehre maybe should be some check if the location give sense. But The jefilechooser is probably handling this.
This UI code should not check the location.getText()'s value. If anyone were to blame, it should be the job of the configuration loader/saver. And, as you have mentioned correctly JFileChooser will either not get constructed when its parameter value is not a correct path or it is going to be constructed with JFileChooser's default path (e.g. "My documents" on Windows or "~" on Un*x systems).
>
> > fileChooser.setFileSelectionMode(JFileChooser.DIRECTORIES_ONLY);
> > fileChooser.setFileHidingEnabled(false);
> > - if (fileChooser.showOpenDialog(null) == JFileChooser.APPROVE_OPTION) {
> > + fileChooser.setAcceptAllFileFilterUsed(false);
>
> Why this? Isn't selection restricted enough by JFileChooser.DIRECTORIES_ONLY ?
Because JFileChooser does not actually present a pure directory-chooser (like in Win32 API for example) when JFileChooser.DIRECTORIES_ONLY is set. The usual (swing) JFileChooser for files is being displayed. And, although AllFilesFilter does not present files even when selected it is simply misleading in this case. As always, the rule of thumb is to display as little UI elements as possible but enough as necessary only.
> > + fileChooser.setDialogTitle(Translator.R("TIFPLocationLabel"));
> > + if (fileChooser.showDialog(null, Translator.R("TIFPFileChooserChooseButton")) == JFileChooser.APPROVE_OPTION) {
> > // Check if we have permission to write to that location.
> > String result = fileChooser.getSelectedFile().getAbsolutePath();
> > File dirLocation = new File(result);
> > @@ -242,4 +244,4 @@
> >
> > config.setProperty(properties[2], spCacheSize.getValue().toString());
> > }
> > -}
> > +}
> > No newline at end of file
> > diff -r f1eaa1ee7891 netx/net/sourceforge/jnlp/resources/Messages.properties
> > --- a/netx/net/sourceforge/jnlp/resources/Messages.properties
> > +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties
> > @@ -291,7 +292,7 @@
> > CVUser=User
> > CVSystem=System
> >
> > -#KeyStores: see KeyStores.java
> > +# KeyStores: see KeyStores.java
> > KS=KeyStore
> > KSCerts=Trusted Certificates
> > KSJsseCerts=Trusted JSSE Certificates
> > @@ -461,6 +462,7 @@
> > TIFPCacheSize=Set the amount of disk space for storing temporary files
> > TIFPDeleteFiles=Delete files
> > TIFPViewFiles=View files...
> > +TIFPFileChooserChooseButton=Choose
>
>
> It will be "Vybrat"
Thank you, I am going to add this since it is no big issue.
> > # Control Panel - Cache Viewer
> > CVCPDialogTitle=Cache Viewer
> > diff -r f1eaa1ee7891 netx/net/sourceforge/jnlp/resources/Messages_cs.properties
> > --- a/netx/net/sourceforge/jnlp/resources/Messages_cs.properties
> > +++ b/netx/net/sourceforge/jnlp/resources/Messages_cs.properties
> > @@ -278,6 +278,7 @@
> > CVUser=Uu017eivatel
> > CVSystem=Systu00e9m
> >
> > +# KeyStores: see KeyStores.java
> > KS=u00dalou017eiu0161tu011b klu00edu010du016f
> > KSCerts=Du016fvu011bryhodnu00e9 certifiku00e1ty
> > KSJsseCerts=Du016fvu011bryhodnu00e9 certifiku00e1ty JSSE
> > diff -r f1eaa1ee7891 netx/net/sourceforge/jnlp/resources/Messages_de.properties
> > --- a/netx/net/sourceforge/jnlp/resources/Messages_de.properties
> > +++ b/netx/net/sourceforge/jnlp/resources/Messages_de.properties
> > @@ -193,9 +194,10 @@
> > BOAbout=Zeigt eine Beispielanwendung.
> > BONosecurity=Deaktiviert die sichere Laufzeitumgebung.
> > BONoupdate=Deaktiviert die Pru00fcfung nach Aktualisierungen.
> > -BOHeadless=Deaktiviert Herunterladefenster, andere Benutzeroberflu00e4chen.
> > +BOHeadless=Deaktiviert Herunterladefenster und anderen Benutzeroberflu00e4chen.
>
>
> hmhmh, this.. "anderen Benutzeroberfl" loong space is caused by reason? Was it intentional split by "n" or unwilling action?
> I would rather prefer single lines, as it is already an custom in itw properties.
The reason for adding those spaces is because these messages are meant or supposed to be limited to 72 or 80 (don't recall the exact number while writing this email anymore) chars, but some do not fit this limit and have to be extended nicely. It simply looks cleaner when long messages are wrapped manually on that boundry instead of bluntly continuing without perhaps proper hyphenation.
> > BOStrict=Aktiviert die strikte Pru00fcfung des JNLP-Dateiformats.
> > BOViewer=Zeigt die Ansicht der vertrauenswu00fcrdigen Zertifikate.
> > +BOXml=Verwendet einen strikten XML-Parser fu00fcr die JNLP-Datei.
> > BXnofork=Keine weitere JVM erstellen.
> > BXclearcache=Den JNLP-Anwendungszwischenspeicher su00e4ubern.
> > BXignoreheaders=Die Pru00fcfung der Metadaten von Jar-Dateien auslassen.
> > @@ -285,6 +287,7 @@
> > CVUser=Benutzer
> > CVSystem=System
> >
> > +# KeyStores: see KeyStores.java
> > KS=Schlu00fcsselspeicher
> > KSCerts=Vertrauenswu00fcrdige Zertifikate
> > KSJsseCerts=Vertrauenswu00fcrdige JSSE Zertifikate
> > @@ -454,6 +457,7 @@
> > TIFPCacheSize=Menge des Plattenplatzes zur Speicherung temporu00e4rer Dateien
> > TIFPDeleteFiles=Dateien lu00f6schen
> > TIFPViewFiles=Dateien anzeigen...
> > +TIFPFileChooserChooseButton=Auswu00e4hlen
> >
> > # Control Panel - Cache Viewer
> > CVCPDialogTitle=Zwischenspeicheranzeige
> > diff -r f1eaa1ee7891 netx/net/sourceforge/jnlp/resources/Messages_pl.properties
> > --- a/netx/net/sourceforge/jnlp/resources/Messages_pl.properties
> > +++ b/netx/net/sourceforge/jnlp/resources/Messages_pl.properties
> > @@ -196,6 +197,7 @@
> > BOHeadless=Wyu0142u0105cza okno pobierania i inne interfejsy graficzne
> > BOStrict=Wu0142u0105cza u015bcisu0142e sprawdzanie format pliku JNLP
> > BOViewer=Pokazuje podglu0105d zaufanych certyfikatu00f3w
> > +BOXml=Stosuje u015bcisu0142y analizator sku0142adniowy XML do analizyn pliku JNLP
> > BXnofork=Nie twu00f3rz nastu0119pnej JVM
> > BXclearcache=Wyczyu015bu0107 pamiu0119u0107 podru0119cznu0105 aplikacji JNLP
> > BXignoreheaders=Pomijaj weryfikacju0119 nagu0142u00f3wku00f3w pliku00f3w jar
> > @@ -285,6 +287,7 @@
> > CVUser=Uu017cytkownik
> > CVSystem=System
> >
> > +# KeyStores: see KeyStores.java
> > KS=Baza kluczy
> > KSCerts=Zaufane certyfikaty
> > KSJsseCerts=Zaufane certyfikaty JSSE
> > @@ -454,6 +457,7 @@
> > TIFPCacheSize=Nastaw wielkou015bu0107 miejsca na dysku do sku0142adowania pliku00f3w tymczasowych
> > TIFPDeleteFiles=Usuu0144 pliki
> > TIFPViewFiles=Przeglu0105daj pliki...
> > +TIFPFileChooserChooseButton=Wybierz
> >
> > # Control Panel - Cache Viewer
> > CVCPDialogTitle=Podglu0105d pamiu0119ci podru0119cznej
>
> Thank you very much to stitch with it and sorry for nits!
No problem and thank you for reviewing. After all, I will take this as a "yes to push".
Regards,
Jacob
More information about the distro-pkg-dev
mailing list