Thanks!

-- Jon

On 3/23/20 1:21 PM, Pavel Rappo wrote:
This latter webrev looks good to me. Thanks.

-Pavel

On 20 Mar 2020, at 16:20, Jonathan Gibbons <[email protected]> wrote:

Updated webrev, with updated test.

The update to the test is intentionally simple, with repeated scans with 
different regex, instead of trying to write a single big pattern, which would 
then result in issues trying to get the right group match.

I note the test confirms that it has detected the new pattern, as seen in this 
fragment of the .jtr file.  i.e. note that result-item is now in the list.

Starting subtest 1.36: Checking CSS classes found
Passed: 5 found: [.result-highlight, .result-item, .search-tag-desc-result, 
.search-tag-holder-result, .ui-autocomplete-category]
-- Jon

Webrev: http://cr.openjdk.java.net/~jjg/8241292/webrev.01/index.html


On 3/20/20 8:04 AM, Jonathan Gibbons wrote:
Thanks for the response; I'll check out "resultItem".

-- Jon

On 3/20/20 3:56 AM, Pavel Rappo wrote:
Hi Jon,

The proposed patch does fix the issue in question. That said, I noticed one more
visual difference between the current L&F and that of before JDK-8240916:
the font size of search result items.

Long story short, there's one more class name we forgot to change, "resultItem".
I'm not sure though how to update the proposed test to cover for that case too.

Otherwise, looks good.

-Pavel

On 19 Mar 2020, at 21:08, Jonathan Gibbons <[email protected]> wrote:

Please review a very simple fix for the reported regression. The fix is just to 
update the search.js file with the hyphenated class names.

Since there is no compile-time check between the contents of a JavaScript file 
and the contents of a stylesheet file, I've updated TestSearch.java to check 
that any CSS class names referenced in the JavaScript file also appear in the 
stylesheet file.

Note: there is another old-style stylesheet files with camelCase names in the 
test directory, that could be updated at some point.
open/test/langtools/jdk/javadoc/doclet/testOptions/custom-stylesheet.css
This file exists to help test options, and the content does not appear to be 
important.  Updating it can be done separately.

-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8241292
Webrev: http://cr.openjdk.java.net/~jjg/8241292/webrev.00/index.html


Reply via email to