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

[Jiri Vanek]

On 01/27/2014 07:29 PM, Jacob Wisor wrote:
> 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.

nn :). I hoped for some more tolerant thing as xmllint adapted for xhtml (or better javadoc). I 
wonted to have it  in automated run as make test-xtml (and test return 0/!0)

nvm. It was just idea.


javadoc itself - is far away from  bein perfect xhtml :(	
>
>> 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.

fair enough. Thanx.
>
>>   /**
>> - *
>> - * @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.

hmhhm. The oldest classes (original NETX ones) have author and email-to. Since icedtea take cover 
over project, the author and email-to become obsoleted (as distro-pkg-dev is email now)

I would like to remove, just what was in this hunk - me as Author in this class, as I joined the 
team long after icedtea-web was under icedtea.

If you do have some concerns about my arguments here. Please keep this hunk as you wish.

>
>> 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.
>
hhmm. I have just tested. I have not spotted the "strange behavior" which I had during development.
So its probably ok.... Do you mind to add this hunk as separate chnageset?

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

message properties are ok.
>
>> 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.

Only what will pass without much issues.

J.