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

Thu Feb 11 17:53:05 UTC 2021

> 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 three additional commits since the last revision:

 - 8260687: Adjust BodyInheritedFontSize test

   -   Add <p style="font-size: 100%">...</p> element/case

       It is important to have it before the <p>...</p> one, so it could
       trigger the pre-existing problem;

   -   Replace StyleConstants.getFontSize() for GlyphView.getFont().getSize()

       The former is unreliable as it doesn't explicitly send a StyleSheet
       context and depends on generally unknown state of the
       CSS.styleSheet.w3cLengthUnits the FontSize declaration has been
       instantiated from.  At the end it doesn't report the actual font
       set to the view.
 - Run TestWrongCSSFontSize in W3C_LENGTH_UNITS-enabled mode too
 - Automatic test for JDK-8260687: Inherited font size is smaller than expected when using StyleSheet to add styles

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2515/files
  - new: https://git.openjdk.java.net/jdk/pull/2515/files/0fb02638..cbab2ca5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2515&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2515&range=00-01

  Stats: 189 lines in 2 files changed: 181 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2515.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2515/head:pull/2515

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