RFR: 8215307: Pages do not have <h1>

Tue Feb 12 21:22:17 UTC 2019

Hannes,

I confirm the effects that I think you observed; I will fix.

-- Jon

On 01/29/2019 03:50 AM, Hannes Wallnöfer wrote:
> Hi Jon,
>
> The patch looks good to me.
>
> Regarding changes (or lack of changes) in the stylesheet: I think it is ok to not adapt header font size, but there is a font-style:italic rule for h3 headers which I think is problematic. That rule has moved from details heading to member heading in type pages, except that it is overwritten *sometimes* but not always by the following rule:
>
> ul.blockList ul.blockList ul.blockList li.blockList h3 {
>      font-style:normal;
> }
>
> That rule for some reason does not apply to the last member in each category (i.e. the last field, constructor, method of a type), so that those member headers are displayed in italic font.
>
> I think the proper solution would be to move the font-style:italic rule from h3 to h2 (so summary and details headings would still be displayed in italic), but I’m not quite sure how that might affect other pages.
>
> Hannes
>
>
>
>> Am 25.01.2019 um 02:24 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
>>
>> This is a fix for the long-standing problems concerning "gaps" (and related problems) in the headings (h1..h6 tags) in generated pages, that show up as accessibility issues.
>>
>> The problem was initially reported in the context of <h1> being missing from the documentation pages for type declarations. In the course of the work, an additional problem was discovered in the set of headings used in the serialized-form page. This fix should address all such problems.
>>
>> The fix comes in various parts.
>>
>> src/ changes ...
>>
>> 1. Most headings are generated using constants defined in a poorly-named class called HtmlConstants, in a poorly-chosen package (html.markup.) The constants are not very well defined, and it is hard to determine which constant should be used in which context. In particular, there are cases where the same constant is used in different contexts that should require different heading levels, and that helps contribute to the root cause of the problem. So, the constants are moved and reorganized into a new class specifically for heading constants (Headings.java), using nested classes to group the constants for each page. There are two notable nested classes, one for type declarations and another for the serialized-form page, which are the two kinds of pages with the most complex heading structure, and (not surprisingly) the most bugs.  Additional nested classes are added for other notable contexts, even if they only have a singleton entry. This will allow those contexts to be extended if the need arises. Within the Headings.TypeDeclaration class, the headings are adjusted to start from h1 instead of h2, which is the root of the fix to the issue. Iwent back and forth on the naming of the constants ... should they end in _HEADING or not? I tries both with and without. Right now, I left the names long; it works OK.
>>
>> 2. With the headings removed from HtmlConstants, the remaining constants are mostly a set of "marker comments" and a single constant for the default charset. The constant for the default charset is moved to the one class where it is used (HtmlConfiguration), leaving the HtmlConstants file to be renamed to MarkerComments.java, since that is all that remain.
>>
>> 3. Most of the remaining changes in the src/ directory are just updates to use the new reorganized Headings. There was one complication that occurred in two files, where the sequence of headings depended on whether or not the user provided options on the command line! Both instances are in files related to the "Frames" feature, which is scheduled for removal soon, and so I just "hacked" a solution to use the right header depending on the circumstances. If we were not planning to remove support for frames, it might have been worth figuring a different solution to the generated output, but that did not seem like a worthwhile effort.
>>
>> test/ changes ...
>>
>> As was to be expected, changing the headings for type declarations broke a number of tests which included a heading as part of their golden output. Since it is not easy to discern the correct values in all cases in all files just by looking at the fragments of code, the dominant most important test is the first change in JavadocTester, to globally enable by default the recently added accessibility checker, which checks for missing headings.  With that check enabled, it was possible to iterate between fixing tests and fixing source code, to ensure that there are no longer any missing headings.
>>
>> While in JavadocTester, I did some cosmetic cleanup to the doc comments in that file.
>>
>> One problem area was the Serialized Form page, which was visibly OK on the screen, but structurally bad when you looked at the headings. (i.e. CSS was papering over the problem.) For this problem, it was useful to add another new feature to JavadocTester, to print out a filtered view of just the headings in a page. While the check for missing headings finds many problems, it doesn't find all. For example, if a page should have headings "H1 H2 H3" but actually has "H1 H2 H2" that will pass the "missing headings" test, even though the page is logically incorrect. The new ShowHeadings feature in JavadocTester helps to expose such problems by making it easy to (manually) verify the hierarchy of headings in a file.
>>
>> stylesheet changes ...
>>
>> The standard javadoc stylesheet uses almost-global settings for the font size and style of the 6 levels of headings. For the most part, these settings are used across all pages. (The exceptions are for the index files in the frames feature, which, as has already been noted, is going away sometime soon.)  The one additional visual cue for headings is the occasional use of a grey background for headings, which does vary depending on the context (i.e. page and heading-level.)
>>
>> The only change to the stylesheet at this point is to ensure that the grey background is used in the same positions as before ... typically for the heading that precedes a list of elements, such as a list of members in a class. The global settings for the font size and style are (intentionally) not modified in this changeset.  This means that the appearance of headings is generally consistent across all kinds of pages, although there are some visual changes in some of the generated pages . We could revisit that if necessary to get the same appearance as before, meaning that we would be visually inconsistent between different kinds of pages. If so, that should be a different cleanup task. I note that it would be a CSS-only task; I believe there is enough info in the HTML that no further changes to the generated HTML would be necessary.
>>
>> In various discussions regarding this feature, concerns were expressed that "<h1> is too big".  Given the existing definitions within the standard stylesheet, this does not seem to be a problem in practice.
>>
>>
>> Review tips:
>>
>> * Start by looking at how HtmlConstants was converted into Headings and MarkerComments
>> * For tests, start by looking at JavadocTester
>> * For the mundane changes to tests, you may find it worth looking at the patch files instead of the side-by-side diffs
>>
>> -- Jon
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8215307
>> Webrev: http://cr.openjdk.java.net/~jjg/8215307/webrev.00/
>>