On Tue, 5 Oct 2021 17:20:25 GMT, Hannes Wallnöfer <[email protected]> wrote:
>> Please review a change to use the HTML `placeholder` attribute in the >> javadoc search input instead of an actual input value. This revives the fix >> I originally created for JDK-8221366 but forwent back then because of >> insufficient support by the Microsoft IE and Edge browsers. MS Edge has >> since been updated and supports the placeholder attribute well now. >> >> Example pages generated with this change can be viewed here (top level pages >> only): >> http://cr.openjdk.java.net/~hannesw/8274625/api.00/ >> >> I changed the color of the placeholder to a lighter grey than we used for >> our own solution because the placeholder attribute works in a slightly >> different way. While we removed the placeholder text when the user clicked >> on the input field, the standard placeholder is still visible right until >> the user starts entering text. Because of this, using a darker color for the >> placeholder is confusing because it can easily be mistaken for actual input. >> The lighter grey is also closer to the default value for most browsers and >> web frameworks such as bootstrap. >> >> I also changed the HTML for the search input to not have an initial value of >> "search" which had then to be cleared by the search script. Furthermore, the >> `disabled` attributes now come in HTML5 value-less boolean format, i.e. >> `disabled` instead of `disabled="disabled"`. >> >> In addition to most significant desktop browsers I tested the generated docs >> on various mobile browsers on Android and iOS devices. > > Hannes Wallnöfer has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains three commits: > > - Merge branch 'master' into JDK-8274625 > - JDK-8274625: Do not set initial value attribute on search input > - JDK-8274625: Search field placeholder behavior We should use standard tools for standard tasks where possible. In this case using a "placeholder" instead of the custom "watermark" workaround fixes the problem, simplifies the code and looks better. Update the copyright years before integrating. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 604: > 602: .put(HtmlAttr.PLACEHOLDER, searchPlaceholder); > 603: HtmlTree inputReset = HtmlTree.INPUT(reset, HtmlIds.RESET_BUTTON) > 604: .put(HtmlAttr.VALUE, reset); Isn't it a mere coincidence that "reset" is both an input type and the value of the "value" attribute: <input type="reset" id="reset-button" disabled value="reset"> Why do we need this "value" attribute? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 534: > 532: .put(HtmlAttr.TYPE, type) > 533: .setId(id) > 534: .put(HtmlAttr.DISABLED, ""); I'm curious: if such a call to `put` translates to a boolean HTML attribute, what call to `put` translates to an HTML attribute with an empty value? src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties line 226: > 224: doclet.Implementation=Implementation(s): > 225: doclet.search=SEARCH: > 226: doclet.search_placeholder=Search Later we should consider a better placeholder, such as an example input. We already have the adjacent "SEARCH" text label and the magnifying glass icon. Something tells me that the user has a pretty good idea what this field is for and that they don't need yet another "Search". ------------- Marked as reviewed by prappo (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5825
