RFR: JDK-8207214: Broken links in JDK API serialized-form page.

[Jonathan Gibbons]

Hannes,

Thanks for looking at this, and for your questions.  Answers inline.

-- Jon


On 07/20/2018 08:24 AM, Hannes Wallnöfer wrote:
> Hi Jon,
>
> I don’t quite all aspects of this fix, which is why I didn’t send a review in the first place. But since nobody else did either, I’ll just ask a few questions to help me better understand.
>
> My main area of confusion is related to what is considered linkable and what not. This should be well-defined, but from your description below (saying this is just a partial fix) it doesn’t seem to be.

Yes, it's not well-defined, at least not in any moderately formal 
documentation.  This is another area in which javadoc uses and reuses 
low-level constructs without wrapping them in higher level abstractions. 
The point about this just being a partial fix was because I see the new 
method isLinkable to be the place to encapsulate what it means to be 
linkable, where "linkable" means "the ability the provide a non-broken, 
clickable link to a useful and relevant target."  As we investigate more 
broken links in the docs, we may see the new method being enhanced to 
accommodate that cases.

> Specific to your webrev, why are members of undocumented super types considered linkable?

This is another undocumented "quirk" of javadoc, that is reasonable in 
hindsight when you think about it.  If a package-private superclass has 
public methods, those public methods can be invoked by a client using a 
public subtype. The question then becomes, where should those methods be 
documented?  If the immediately enclosing type is undocumented, then the 
first public subtype becomes a "reasonable" place to document the 
methods. Maybe there are other "reasonable" ways to present the 
information, but this is the current (long-term) behavior of the doclet.

Kumar found this behavior (the hard way) when writing the new 
VisibleMemberTable, which replaces the old VisibleMemberMap. This sort 
of behavior is the primary reason for comparing "before" and "after" 
versions of generate docs when replacing or updating javadoc internals. 
"If you don't think you're making any changes to the output, verify 
you're not making any changes to the output!"

>
> I also looked at the serialised form page of the current API docs and didn’t find the broken links mentioned in the JIRA issue, so I wasn’t able to observe the impact of the fix.

Most links on the serialized form page are OK.  Serialization is "weird" 
because it includes the doc for private fields, and in a number of 
places, the doc comment on the private fields contains @see references 
to other private members which are not being documented.  It is those 
links which were broken and which now should not be broken, at least 
from a clickable-link perspective. It is still "not great" that the 
doclet posts an (unlinked) reference to an undocumented member.  It 
could be an RFE to improve that, but while I can imagine better behavior 
of @see links, I have no good suggestion for the equivalent inline 
{@link} tags.

Here is an example of the broken link:

In the JDK 10 API docs, look at the Serialized form for java.awt.MenuItem:
https://docs.oracle.com/javase/10/docs/api/serialized-form.html#java.awt.MenuItem
Look at the last entry in the See Also list: 
|MenuItem.writeObject(ObjectOutputStream)| 
<https://docs.oracle.com/javase/10/docs/api/java/awt/MenuItem.html#writeObject%28java.io.ObjectOutputStream%29>
If you click on it you go to the page for MenuItem, but because there is 
no id for the writeObject method, you just go to the top of the page.

Now look at the corresponding page in the docs with this fix:
http://cr.openjdk.java.net/~jjg/8207214/api.00/serialized-form.html#java.awt.MenuItem
Look at that same last entry again. This time, it is just plain text, 
not linked.

Arguably, a better solution for a followup RFE would be to link the 
class name (because the class is documented) but not the member name. 
But that is a more tricky fix.

-- Jon

>
> Apart from that, the webrev looks good to me.
>
> Hannes
>
>
>> Am 13.07.2018 um 02:16 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
>>
>> Please review a small but slightly tricky fix related to broken links on the serialized-form page.
>>
>> The underlying problem was that the doclet was generating bad links when an @see tag
>> was referencing a member that was not itself being documented. For example, the following
>> will normally cause a broken link because privateMethod will not normally be included in the
>> documentation.
>>
>> public class C {
>>    /** Text.
>>      * @see #privateMethod()
>>      */
>>     public void publicMethod() { }
>>
>>     private void privateMethod() { }
>> }
>>
>>
>> There is a spectrum of possible solutions, some more complicated than others.
>> The approach here is to fix the perceived original intent of the code which is to detect
>> when the link will not be generated, and in that case, just write the label, without
>> any enclosing link.
>>
>> This is done by creating and using a new method in Utils that carefully tries to determine
>> whether the link should be generated. It's not ideal, and may need to be refined in due course,
>> as we identify more test cases, but it is arguably "good enough" for now, in that all existing
>> tests continue to pass, as well as some new test cases that mimic the problem as originally
>> discovered.  It is possible that we might be able to simplify this code, in time, by using
>> information in the recently-new VisibleMemberTable.
>>
>> With regard to the original problem of broken links on the serialized-form page in the
>> JDK API, the number of broken links has gone down by about 15%.
>>
>> With respect to the webrev, there is some minor cleanup, but primarily, the expression
>> on HtmlDocletWriter:916 has been replaced by a call to a new method in Utils, that
>> provides a more nuanced determination of whether the link should be generated
>> or not.
>>
>> The new test creates various test scenarios and runs them through javadoc. It is worth
>> noting that by default *all* javadoc tests using JavadocTester now run a link checker
>> (checkLinks) unless it is explicitly disabled, and that ensures no regressions in the
>> output generated in any other of the javadoc tests.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8207214
>> Webrev: http://cr.openjdk.java.net/~jjg/8207214/webrev.00/index.html
>>
>> -- Jon
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20180723/afe7a132/attachment.html>