[rfc][icedtea-web] Javadoc, XHTML conformance, and formatting cleanup

[Jacob Wisor]

On 01/27/2014 03:22 PM, Jiri Vanek wrote:
> On 01/27/2014 05:43 AM, Jacob Wisor wrote:
>> Hello,
>>
>> The subject says it. I was especially bothered by javadoc throwing @doctag
>> errors and warnings as
>> well as its output not being XML conform. Many HTML elements that have
>> gathered over time in the
>> source code have not been XHTML conform hence causing javadoc's output also
>> not being XML conform. I
>> think this patch could serve as an improvement to the overall quality of the
>> source code as the
>> world transitions to strictly XML conforming data. So, the HTML documents
>> generated by javadoc
>> should be fully conforming to XML, hence XHTML documents now. Yey! :-)
>>
>> Jacob
>
>
> Wou. Thats the patch.
>
>
> Have the generated javadoc passed some check after the improvements?

Nothing in particular than the obvious. Javadoc just does not complain anymore 
and it still compiles. I did not use any external validators.

> If so What it was, I would include it into some tests-javadoc target...

You could do that, or perhaps the developer's IDE would be a better place for 
it? You know, eg. Eclipse has extensive formatting and javadoc features 
reminding developers of style etc. Anyway, I suppose you would have to test for 
@doctag (fairly easy) and XHTML conformance. Especially the latter one could 
become a problem because the test is probably going to depend on a validator 
library. The W3C has only since recently provided some public web driven 
validators, so perhaps writing the test would be as easy as opening a 
HttpConnection and posting the javadoc output on a per page basis. But then 
perhaps, some automated build servers may run into problems while not being 
allowed to access the internet... Perhaps you could reuse those Eclipse 
libraries with adequate style configuration? I do not know for sure, but that is 
what you would have to figure out.

> Few comments to changes:
>
> You are using
>
> + * <pre><code>
>    * FirefoxPreferencesParser p = new FirefoxPreferencesParser(prefsFile);
>    * p.parse();
>    * Map&lt;String,String&gt; prefs = p.getPreferences();
>    * System.out.println("blink allowed: " + prefs.get("browser.blink_allowed"));
> - * </pre>
> + * </code></pre>
>
> pattern few times - Whats the benefitof it? I would say pre xor code is enough,
> isnt it?

<pre> is just a styling element, giving a hint as how to render enclosed 
content. <code> is a meta element meaning that the enclosed content is 
considered some sort of code that could serve as input to some parsers or 
compiles etc. <code> does not carry any notion about rendering.

>   /**
> - *
> - * @author jvanek
> + * @author <a href="mailto:Jiri%20Vanek%20&lt;jvanek at redhat.com&gt;">Jiri
> Vanek</a>
>    */
>
> Ugh. Author tag is not allowed. Please just remove instead of fix.

Why is that? A lot of classes have @author doctags. It does not denote 
ownership. It is just a hint as to whom to turn for questions about the code in 
the future.
If they are absolutely not allowed, well then all other existing @author doctags 
need to be stripped too. And, that's kind of mind boggling.

> diff -r efa527f74184
> netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPaneModel.java
> --- a/netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPaneModel.java
> +++ b/netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPaneModel.java
> @@ -188,8 +188,8 @@
>               }
>               String line = (createLine(messageWithHeader));
>               if (mark) {
> -                line = line.replaceAll("\n", "<br>\n");
> -                line = line.replaceAll("  ", "&nbsp; ");//small trick, html is
> reducting row of sapces to single space. This handles it and stimm allow line wrap
> +                line = line.replaceAll("\n", "<br/>\n");
> +                line = line.replaceAll("  ", "&nbsp; ");//small trick, html is
> reducting row of spaces to single space. This handles it and stimm allow line wrap
>                   line = line.replaceAll("\t", "&nbsp;&nbsp;&nbsp;&nbsp;");
>
> This is not possible. Unluckily the java html parser do not survive <br/>.
> Please remove this hunk.

That's a drag. :-\ It should definitely accept it. I guess the same applies to 
some message properties?

> After fixing above. Looks ok.
>
>
> Is backport to 1.4 possible? As the cnages are 99% really javadoc related, I do
> not insists. My motivation  to backport have only one  valaid point - The
> refactoring is changing  backporting and forwardporting  from automatic level to
> manual one.  Unless you have something against, do you mind to backport valid
> parts after  1.4.2 release?

I do know for sure because there is a lot of stuff here that is not in 1.4. But, 
I will take a look at it.

Jacob