On Fri, 12 Feb 2021 16:29:07 GMT, Stanimir Stamenkov 
<github.com+1247730+sta...@openjdk.org> wrote:

>> Proposed fix for [JDK-8260687][].
>> 
>> It is noted the issue appeared after the [JDK-8257664][] (PR #1759) fix 
>> landed but the underlying problem is present even before that as 
>> demonstrated by using the following HTML document variant:
>> 
>> <html>
>> <body>
>>   <p style="font-size: 100%">16pt inherited from body</p>
>>   <p style="font-size: 16pt">16pt paragraph</p>
>> </body>
>> </html>
>> 
>> with the program setup given in the ticket.  The PR #1759 fix basically does 
>> just that – implies `font-size: 100%` where no `font-size` specified 
>> explicitly to ensure correctly computed font-size gets inherited when an 
>> ancestor specifies a percentage font-size different from `100%` (or `1em`) – 
>> otherwise the percentage is calculated and multiplied at every level of the 
>> inheritance, producing wrong results.
>> 
>> ---
>> 
>> The underlying problem is probably a combination of inconsistencies one may 
>> observe with the complex implementation of the 
>> [`W3C_LENGTH_UNITS`](https://docs.oracle.com/en/java/javase/15/docs/api/java.desktop/javax/swing/JEditorPane.html#W3C_LENGTH_UNITS)
>>  editor property.  Starting with only the `HTMLDocument.styleSheet` gets its 
>> internal `StyleSheet.w3cLengthUnits` field set up according to the 
>> `JEditorPane.W3C_LENGTH_UNITS` property.  Thus when a `font-size: 16pt` rule 
>> is part of the `HTMLEditorKit.styleSheet` which `w3cLengthUnits` generally 
>> remains `false` – querying the font for it from that style sheet yields a 
>> different result from querying the same rule via the 
>> `HTMLDocument.styleSheet`:
>> 
>>         JEditorPane editor = new JEditorPane();
>>         editor.putClientProperty(JEditorPane.W3C_LENGTH_UNITS, true);
>> 
>>         HTMLEditorKit htmlKit = new HTMLEditorKit();
>>         editor.setEditorKit(htmlKit);
>> 
>>         HTMLDocument htmlDoc = (HTMLDocument) editor.getDocument();
>> 
>>         StyleSheet kitStyles = htmlKit.getStyleSheet();
>>         StyleSheet docStyles = htmlDoc.getStyleSheet();
>>         assert Arrays.asList(docStyles.getStyleSheets()).contains(kitStyles);
>> 
>>         System.out.println(docStyles.getFont(docStyles.getRule("body"))); // 
>> [1]
>> 
>>         kitStyles.addRule("body { font-family: sans-serif; font-size: 16pt; 
>> }");
>> 
>>         System.out.println(kitStyles.getFont(kitStyles.getRule("body"))); // 
>> font.size = 16 (subjectively not expected)
>>         System.out.println(docStyles.getFont(docStyles.getRule("body"))); // 
>> font.size ≠ [1] ≠ 16 (expected)
>> 
>> That's probably fine as one may programmatically create and add more style 
>> sheets to the `HTMLDocument.styleSheet`.  These style sheets may further 
>> turn out shared with multiple documents/editors with different 
>> `W3C_LENGTH_UNITS` settings.
>> 
>> The next issue with the `W3C_LENGTH_UNITS` implementation, this PR proposes 
>> a fix for, is related to the current implementation of 
>> `StyleSheet.ViewAttributeSet.getAttribute()`:
>> 
>> https://github.com/openjdk/jdk/blob/70b5b3119b2ed032b021a080f34a4fa28d092fb5/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java#L2810-L2818
>> 
>> which in turn invokes `CSS.FontSize.toStyleConstants()`:
>> 
>> https://github.com/openjdk/jdk/blob/48c932e1f1e5a79a28211f72dc9f10d8fd30b955/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2195-L2199
>> 
>> One may notice the latter loses the `StyleSheet` context necessary to 
>> properly resolve `w3cLengthUnits` from `CSS.FontSize.getValue()`:
>> 
>> https://github.com/openjdk/jdk/blob/48c932e1f1e5a79a28211f72dc9f10d8fd30b955/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2049-L2061
>> 
>> This context is properly sent from `StyleSheet.getFont()`, for example:
>> 
>> https://github.com/openjdk/jdk/blob/70b5b3119b2ed032b021a080f34a4fa28d092fb5/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java#L913-L914
>> 
>> ---
>> 
>> I'll further add a test based on `BodyInheritedFontSize.java` already given 
>> to [JDK-8260687][] – I just want to enhance it with the:
>> 
>>   <p style="font-size: 100%">16pt inherited from body</p>
>> 
>> testing the issue from a different angle as noted at the beginning of this 
>> PR description.
>> 
>> [JDK-8257664]: https://bugs.openjdk.java.net/browse/JDK-8257664 
>> "HTMLEditorKit: Wrong CSS relative font sizes"
>> [JDK-8260687]: https://bugs.openjdk.java.net/browse/JDK-8260687 "Inherited 
>> font size is smaller than expected when using StyleSheet to add styles"
>
> Stanimir Stamenkov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8260687: Adjust TestWrongCSSFontSize
>   
>   -  Always save image capture to the current working directory on test
>      failure;
>   -  Save successful image capture with '-capture' program argument;
>   -  Always save image capture with ".png" extension;
>   -  Use '-w3cUnits' program argument to control the test.w3cUnits flag.

Marked as reviewed by aivanov (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2515

Reply via email to