<Swing Dev> RFR: 8260687: Inherited font size is smaller than expected when using StyleSheet to add styles [v7]

Fri Feb 12 17:05:40 UTC 2021

On Fri, 12 Feb 2021 16:29:07 GMT, Stanimir Stamenkov <github.com+1247730+stanio at 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