Reviewer needed - fix for Re: Strange behaviour during javaws -about part 2/2
Omair Majid
omajid at redhat.com
Thu Mar 3 09:43:02 PST 2011
On 03/03/2011 05:55 AM, Jiri Vanek wrote:
> This part of patch removes code from about window which required
> all-permissions (code from net.sourceforge.jnlp and hyper-links
> control). It also removes applications tab from about window, and
> replaces it with tabs generated from AUTHORS,NEWS and README.
>
>
This patch effectively removes notes.html and applications.html, right?
> diff2.diff
>
>
> diff -r 0e868f1709f1 ChangeLog
> --- a/ChangeLog Thu Mar 03 09:39:41 2011 +0100
> +++ b/ChangeLog Thu Mar 03 11:52:51 2011 +0100
> @@ -1,3 +1,14 @@
> +2011-03-03 Jiri Vanek<jvanek at redhat.com>
> +
> + * Makefile.am: added generation of htmlfiles from NEWS and AUTHORS and
> + RREADME
> + * extras/net/sourceforge/jnlp/about/Main.java: application tab replaced
> + with new tabs with generated files
> + * extras/net/sourceforge/jnlp/about/Main.java: removed useless (can not
> + work without all-permissions) hypertextlistener and runtime which
> + needed special permissions, and another staf from
> + net.sourceforge.jnlp package
> +
You should make explicitly make a note of methods (like hyperlinkUpdate)
which have been removed, as suggested in [1].
> 2011-03-02 Jiri Vanek<jvanek at redhat.com>
>
> * netx/net/sourceforge/jnlp/runtime/Boot.java: getAboutFile changed to
> diff -r 0e868f1709f1 Makefile.am
> --- a/Makefile.am Thu Mar 03 09:39:41 2011 +0100
> +++ b/Makefile.am Thu Mar 03 11:52:51 2011 +0100
> @@ -296,6 +296,21 @@
> ${INSTALL_DATA} -D $${files} \
> $(NETX_EXTRA_DIST_DIR)/$${files}; \
> done)
> +#generate news and authors to be used by javaws -about
> + cd $(abs_top_builddir)
This should not be required. Make will be running in
$(abs_top_builddir). Besides, each make rule executes in a new shell -
so the next line will not be affected by this cd command (look for other
examples of cd in the Makefile to see how that's done).
> + echo "<HTML><BODY><h1>News</h1><pre>"> ${NETX_EXTRA_DIST_DIR}/news.html
> + cat NEWS>> ${NETX_EXTRA_DIST_DIR}/news.html
This fails in an out-of-tree build. Pleas use $(abs_top_srcdir)/NEWS
instead. This also needs to be done for README and AUTHORS.
> + echo "</pre></BODY></HTML>">> ${NETX_EXTRA_DIST_DIR}/news.html
> +
Please dont add blank spaces within rules.
> + echo "<HTML><BODY><h1>Readme</h1><pre>"> ${NETX_EXTRA_DIST_DIR}/readme.html
> + cat README>> ${NETX_EXTRA_DIST_DIR}/readme.html
> + echo "</pre></BODY></HTML>">> ${NETX_EXTRA_DIST_DIR}/readme.html
> +
> + echo "<HTML><BODY>"> ${NETX_EXTRA_DIST_DIR}/authors.html
> + echo "<h1>Authors</h1><div align="center"><img src="jamIcon.jpg" alt="Jam Icon" width="87" height="84" align="Center"/><br/">> ${NETX_EXTRA_DIST_DIR}/authors.html
> + cat AUTHORS | sed s/$$/"<br>"/>> ${NETX_EXTRA_DIST_DIR}/authors.html
> + echo "</div></BODY></HTML>">> ${NETX_EXTRA_DIST_DIR}/authors.html
> +
> mkdir -p stamps
> touch $@
>
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/Main.java
> --- a/extra/net/sourceforge/javaws/about/Main.java Thu Mar 03 09:39:41 2011 +0100
> +++ b/extra/net/sourceforge/javaws/about/Main.java Thu Mar 03 11:52:51 2011 +0100
> @@ -47,40 +47,43 @@
> import javax.swing.JPanel;
> import javax.swing.JTabbedPane;
> import javax.swing.UIManager;
> -import javax.swing.event.HyperlinkEvent;
> -import javax.swing.event.HyperlinkListener;
>
> -import net.sourceforge.jnlp.Launcher;
> -import net.sourceforge.jnlp.runtime.JNLPRuntime;
>
> -public class Main extends JPanel implements HyperlinkListener {
> +public class Main extends JPanel {
>
> - private final String notes = "/net/sourceforge/jnlp/about/resources/notes.html";
> - private final String apps = "/net/sourceforge/jnlp/about/resources/applications.html";
> - private final String about = "/net/sourceforge/jnlp/about/resources/about.html";
> + private final String notes = "/net/sourceforge/javaws/about/resources/notes.html";
> + private final String about = "/net/sourceforge/javaws/about/resources/about.html";
> + private final String news = "/net/sourceforge/javaws/about/resources/news.html";
> + private final String readme = "/net/sourceforge/javaws/about/resources/readme.html";
> + private final String authors = "/net/sourceforge/javaws/about/resources/authors.html";
> +
Are these indentation changes intentional?
> JTabbedPane tabbedPane;
>
> public Main() throws IOException {
> super(new BorderLayout());
>
> HTMLPanel notesPanel = new HTMLPanel(getClass().getResource(notes));
> - HTMLPanel appsPanel = new HTMLPanel(getClass().getResource(apps));
> HTMLPanel aboutPanel = new HTMLPanel(getClass().getResource(about));
> + HTMLPanel newsPanel = new HTMLPanel(getClass().getResource(news));
> + HTMLPanel readmePanel = new HTMLPanel(getClass().getResource(readme));
> + HTMLPanel authorsPanel = new HTMLPanel(getClass().getResource(authors));
>
> - appsPanel.pane.addHyperlinkListener(this);
> +
>
> tabbedPane = new JTabbedPane();
>
> tabbedPane.add("About NetX", aboutPanel);
> - tabbedPane.add("Applications", appsPanel);
> tabbedPane.add("Notes", notesPanel);
> + tabbedPane.add("News", newsPanel);
> + tabbedPane.add("Readme", readmePanel);
> + tabbedPane.add("Authors", authorsPanel);
>
> tabbedPane.setPreferredSize(new Dimension(550,410));
> add(tabbedPane, BorderLayout.CENTER);
> }
>
> private static void createAndShowGUI() {
> - JNLPRuntime.setExitClass(Main.class);
> +
>
> try {
> UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
> @@ -119,17 +122,5 @@
> });
> }
>
> - public void hyperlinkUpdate(HyperlinkEvent e) {
> - if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED) {
> - URL url = e.getURL();
> -
> - Launcher launcher = new Launcher(
> - JNLPRuntime.getDefaultLaunchHandler());
> - try {
> - launcher.launchBackground(url);
> - } catch (Exception ex) {
> - ex.printStackTrace();
> - }
> - }
> - }
> +
> }
Everything else looks fine to me.
Cheers,
Omair
[1]
http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs
More information about the distro-pkg-dev
mailing list