[rfc][icedtea-web] Removing applications tab in jawas-about

[Jiri Vanek]

On 07/05/2013 10:53 PM, Andrew Azores wrote:
> I made a fresh set of patches attached here because my workspace was getting a little dirty and also out of date. Changelog is the same for each patch file though. The new one, "last.patch," just removes all the old extras.jar stuff essentially. All the more important changes are still in the other patches, same as last time.
>
>
> It's back, I think it does make the Authors page look nicer :)

nice! Please double check after push that images are as should be ( I had some issues with older patch command)
>

...


> Added some more to it, but not sure what else to do here.
>> - some indentation/mark-up both for headless/head"full" versions
>
> Added!


Looks excellent! Two minor nits - I would like to have icedtea-web homepage url and removed license header (sorry) in headless about and.. see  general comments  below
>
>> - centre the dialogue (tehre is utility method for such a stuff)
>
> This too :)

This looks like it is not here (or is not working). The  -  public static void  centerWindowsToCurrentScreen(Window w) { from net.sourceforge.jnlp.util.ScreenFinder should do the trick.
>
>> - what about renaming of Copying into GPLv2 License ? (just $0.02)
>
> I like this suggestion. I've renamed it to just "GPLv2" so that the tab's name isn't too long compared to others.

New about dialog is nearly perfect !-)
>
>> - btw about, Authors, Changelog, News... All the tabs should now be read from resources too.
>>
>> Don't take me wrong, this is nice work and most of above are just small nits and some can be done as separated chnageset.
>>
>> - Big nit - redesign the dialogue completely? So it will be "nice" ?
>> - I'm in favour hardcoded (localisable) main "about" screen and then some better swapping to (more customised to its purpose) three underlying tabs. However, unless you really wont this have low priority definitely should go as separate chnageset.
>> - nitpickers note - see other apps about dialogues :) Itw is really few decades behind....
>
> Changed it up quite a bit in this version but not sure how much better it really is. I think it looks nicer at least. It doesn't handle resizing quite as nicely anymore however and I'm really not a Swing guru (or any kind of UI for that matter) and am not sure what to do about it.

good job!
Dialogue is much nicer, and higligting is perfect.

the only nit is the license header in headless mode.  I'm for dropping it (tbh I have not found in code where it come from :)

>
...ability of this code is quite pure.
>>
>>
>> Anyway this cango in with markup, assuming it works.
>
> This grew a little more now... email addresses, eg <aazores at redhat.com>, get turned into "mailto" links now. Actually since it is HTML-escaped first, it would be "&lt;aazores at redhat.com&gt;" that gets turned into a mailto. Similarly, http(s) URLs are automatically turned into hyperlinks. I've done both of those because I had hyperlinks added to the new about.html, and added the ability to click these in HTMLPanel.java. It seemed natural to have the rest of the tabs also contain clickable content.

A bit?? You are quite powerful make/sed/bash  magican. Feature will prove how bullet proof this code is ;)
You have included some comments, and it is really *appreciated* . Maybe even more comments will be nice (eg split the greate regex on start of target [to several lines with individual comments?]?)
>
>>
>>
.,...
>
> Maybe there would still be a way to localize the content, but I don't know how to do it really. Applying it to the generated HTML sounds sketchy, but applying it to the plaintext beforehand sounds like a pretty major change compared to the current way it works, which as you can see is basically just a bunch of sed commands.

nvm any more:)


New dialogue is really nice!




Still several general comments:
   - drop license header in headless mode. It does not looks nice. Sorry:(
      - the  mentioning of COPYING file is more then enough
   - add icedtea-web homepage url in headless about.
   - you have duplicated the splash image. Tats wrong :) But as you did it, I would recomand to shrink it  (so the source iamge have the same size as result image), fill Iced (by some iced-blue) and make web more dark - visible)
    - my $0.002 +1 for having it a bit bigger then now O:)
   - please update changelog for next round
   - localisation!
         aboutPanel = new HTMLPanel(res_about, "About");
+        authorsPanel = new HTMLPanel(res_authors, "Authors");
+        newsPanel = new HTMLPanel(res_news, "News");
+        changelogPanel = new HTMLPanel(res_changelog, "Changelog");
+        copyingPanel = new HTMLPanel(res_copying, "GPLv2");

  - several tests is depending on about.jar or other affected stuff You will have to fix those:(
    - see:  grep about -ir tests/
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/Main.class");
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/Main.class");
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/resources/about.html");
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/resources/about.html");
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/Main_FOO_.class", false);
tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java:        testResourceCaching("net/sourceforge/jnlp/about/Main_FOO_.class", false);

^^ Here and here ˇˇ you will have to create soem jar via jar and javac api.

tests/test-extensions/net/sourceforge/jnlp/mock/DummyJNLPFile.java:            JAR_URL = new URL("http://icedtea.classpath.org/netx/about.jar");
tests/test-extensions/net/sourceforge/jnlp/ContentReader.java:        //mostly compaling when assassin kill the process about StreamClosed

tests/reproducers/simple/LocalesTest/testcases/LocalesTestTest.java:        "BOAbout",

Thsi one expects the BOAbout to be printed, but it looks like you have dropped this "ssentence"


To code itself I do not have any more objections and we are getting close to finish!


tahnx
   J.