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