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 <[email protected]>:
>
> 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/
>
