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

classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
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"

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

Commit messages:
 - 8260687: Preserve StyleSheet context when evaluating CSS.FontSize value

Changes: https://git.openjdk.java.net/jdk/pull/2515/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2515&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260687
  Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 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
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
On Wed, 10 Feb 2021 17:55:56 GMT, Stanimir Stamenkov <[hidden email]> 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"

Hi @stanio. Thank you for your new contribution. I have not run the build and tests for it yet. However, the suggested fix looks reasonable. I thought it was related to the way `W3C_LENGTH_UNITS` is handled, the behaviour changed based on that flag.

I propose that you copy the changes from my [JDK-8260687 branch](https://github.com/aivanov-jdk/jdk/compare/JDK-8260687). There are two changes:

1. [Run TestWrongCSSFontSize in W3C_LENGTH_UNITS-enabled mode too](https://github.com/aivanov-jdk/jdk/commit/16fdd939a4351519c8bc0ca36154443bd0986854) / [updated TestWrongCSSFontSize.java](https://github.com/aivanov-jdk/jdk/blob/16fdd939a4351519c8bc0ca36154443bd0986854/test/jdk/javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java)
2. [New test BodyInheritedFontSize.java](https://github.com/aivanov-jdk/jdk/blob/f9e997776fe478d99f0bbb6739ded10ec79b4caa/test/jdk/javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java), it consists of several commits.

You can then add another `<p>` to the HTML and modify the test accordingly.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Prasanta Sadhukhan-2
In reply to this post by Stanimir Stamenkov
On Wed, 10 Feb 2021 17:55:56 GMT, Stanimir Stamenkov <[hidden email]> 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"

Fix looks ok, it seems. Please also add the testcase given in JBS, or you can add your own.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
In reply to this post by Stanimir Stamenkov
On Wed, 10 Feb 2021 17:55:56 GMT, Stanimir Stamenkov <[hidden email]> 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"

The fix looks good to me too. It passes the set of the four tests related to the recent fixes:

- javax/swing/text/html/CSS/8231286/HtmlFontSizeTest.java
- javax/swing/text/html/CSS/4765271/bug4765271.java
- javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java
- javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java

I've just updated BodyInheritedFontSize.java on [my JDK-8260687 branch](https://github.com/aivanov-jdk/jdk/tree/JDK-8260687) to include your test case with `font-size: 100%`.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Stanimir Stamenkov
> 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
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Stanimir Stamenkov
> 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: Replace line-endings in BodyInheritedFontSize.java

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

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

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

  Stats: 168 lines in 1 file changed: 0 ins; 0 del; 168 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
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Alexey Ivanov-2
On Thu, 11 Feb 2021 16:32:36 GMT, Alexey Ivanov <[hidden email]> wrote:

>> Stanimir Stamenkov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8260687: Replace line-endings in BodyInheritedFontSize.java
>
> The fix looks good to me too. It passes the set of the four tests related to the recent fixes:
>
> - javax/swing/text/html/CSS/8231286/HtmlFontSizeTest.java
> - javax/swing/text/html/CSS/4765271/bug4765271.java
> - javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java
> - javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java
>
> I've just updated BodyInheritedFontSize.java on [my JDK-8260687 branch](https://github.com/aivanov-jdk/jdk/tree/JDK-8260687) to include your test case with `font-size: 100%`.

I have copied the changes from the aivanov-jdk/jdk@f9e997776fe4 branch earlier and made my revision to include the `font-size: 100%` case.  I have the following adjustments that don't appear included in @aivanov-jdk's latest change:

-   The `<p style="font-size: 100%">...</p>` has to be before the `<p>...</p>` to trigger the pre-existing problem (f.e. in Java 11);
-   Replace `StyleConstants.getFontSize()` for `GlyphView.getFont().getSize()`
   
    The former is unreliable as it doesn't explicitly send a `StyleSheet` context and depends on a 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 – it computes a new result that might differ from the actual.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
On Thu, 11 Feb 2021 18:04:26 GMT, Stanimir Stamenkov <[hidden email]> wrote:

>> The fix looks good to me too. It passes the set of the four tests related to the recent fixes:
>>
>> - javax/swing/text/html/CSS/8231286/HtmlFontSizeTest.java
>> - javax/swing/text/html/CSS/4765271/bug4765271.java
>> - javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java
>> - javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java
>>
>> I've just updated BodyInheritedFontSize.java on [my JDK-8260687 branch](https://github.com/aivanov-jdk/jdk/tree/JDK-8260687) to include your test case with `font-size: 100%`.
>
> I have copied the changes from the aivanov-jdk/jdk@f9e997776fe4 branch earlier and made my revision to include the `font-size: 100%` case.  I have the following adjustments that don't appear included in @aivanov-jdk's latest change:
>
> -   The `<p style="font-size: 100%">...</p>` has to be before the `<p>...</p>` to trigger the pre-existing problem (f.e. in Java 11);
> -   Replace `StyleConstants.getFontSize()` for `GlyphView.getFont().getSize()`
>    
>     The former is unreliable as it doesn't explicitly send a `StyleSheet` context and depends on a 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 – it computes a new result that might differ from the actual.

> I have copied the changes from the [aivanov-jdk/jdk@f9e9977](https://github.com/aivanov-jdk/jdk/commit/f9e997776fe4) branch earlier and made my revision to include the `font-size: 100%` case. I have the following adjustments that don't appear included in @aivanov-jdk's latest change:
>
>     * The `<p style="font-size: 100%">...</p>` has to be before the `<p>...</p>` to trigger the pre-existing problem (f.e. in Java 11);

I do not think the order matters. The paragraph which does not specify the font size and the paragraph which specifies it as 100% have the same size. This can be confirmed with another added check, does it make sense?


>     * Replace `StyleConstants.getFontSize()` for `GlyphView.getFont().getSize()`
>       The former is unreliable as it doesn't explicitly send a `StyleSheet` context and depends on a 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 – it computes a new result that might differ from the actual.

Does it not? I can't see it has much of difference: visually the views have different font sizes which is confirmed by the existing check. But the font size in all three cases is expected to be the same.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
In reply to this post by Stanimir Stamenkov
On Thu, 11 Feb 2021 18:02:00 GMT, Stanimir Stamenkov <[hidden email]> 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: Replace line-endings in BodyInheritedFontSize.java

> > I have copied the changes from the [aivanov-jdk/jdk@f9e9977](https://github.com/aivanov-jdk/jdk/commit/f9e997776fe4) branch earlier and made my revision to include the `font-size: 100%` case. I have the following adjustments that don't appear included in @aivanov-jdk's latest change:
> >
> > • The `<p style="font-size: 100%">...</p>` has to be before the `<p>...</p>` to trigger the pre-existing problem (f.e. in Java 11);
>
>
> I do not think the order matters. The paragraph which does not specify the font size and the paragraph which specifies it as 100% have the same size. This can be confirmed with another added check, does it make sense?

Okay, the order does matter, it produces different results. With percentage value first, one more scenario is covered.

The third comparison does not make sense. If the two not-equal-to conditions are false, `fontSizeInherited != fontSizePercentage` is also false.

test/jdk/javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java line 144:

> 142:                     + "Inherited: " + fontSizeInherited + " vs. "
> 143:                     + "Explicit: " + fontSizeExplicit);
> 144:         }

I compared two pairs: `fontSizeInherited != fontSizeExplicit` and `fontSizePrecentage != fontSizeExplicit`. These two checks also imply `fontSizeInherited == fontSizePrecentage` which is also what we want.

Comparing only one pair of the three items is not enough.

Typo: fontSizePrecentage → fontSizePercentage.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
On Thu, 11 Feb 2021 22:13:40 GMT, Alexey Ivanov <[hidden email]> wrote:

> I do not think the order matters...

It doesn't matter just after the #1759 fix.  If you try this with Java 11 or 15 you'll see the problem is present even before that fix.  That's the point.

> Does it not? I can't see it has much of difference: visually...

That's the point, what you see visually is what you get from `GlyphView.getFont()` not necessarily what you get from `StyleConstants.getFontSize()` after the document has been completely laid out and the styles to all views computed.  The latter depends on the last `CSS.styleSheet` state after the last style gets evaluated.  This could be seen again by trying the test by having `<p style="font-size: 100%">` as the first element and Java 11, 15 – the test passes programmatically although visually it obviously failed.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Alexander Zuev-3
On Thu, 11 Feb 2021 22:28:48 GMT, Stanimir Stamenkov <[hidden email]> wrote:

>>> > I have copied the changes from the [aivanov-jdk/jdk@f9e9977](https://github.com/aivanov-jdk/jdk/commit/f9e997776fe4) branch earlier and made my revision to include the `font-size: 100%` case. I have the following adjustments that don't appear included in @aivanov-jdk's latest change:
>>> >
>>> > • The `<p style="font-size: 100%">...</p>` has to be before the `<p>...</p>` to trigger the pre-existing problem (f.e. in Java 11);
>>>
>>>
>>> I do not think the order matters. The paragraph which does not specify the font size and the paragraph which specifies it as 100% have the same size. This can be confirmed with another added check, does it make sense?
>>
>> Okay, the order does matter, it produces different results. With percentage value first, one more scenario is covered.
>>
>> The third comparison does not make sense. If the two not-equal-to conditions are false, `fontSizeInherited != fontSizePercentage` is also false.
>
>> I do not think the order matters...
>
> It doesn't matter just after the #1759 fix.  If you try this with Java 11 or 15 you'll see the problem is present even before that fix.  That's the point.
>
>> Does it not? I can't see it has much of difference: visually...
>
> That's the point, what you see visually is what you get from `GlyphView.getFont()` not necessarily what you get from `StyleConstants.getFontSize()` after the document has been completely laid out and the styles to all views computed.  The latter depends on the last `CSS.styleSheet` state after the last style gets evaluated.  This could be seen again by trying the test by having `<p style="font-size: 100%">` as the first element and Java 11, 15 – the test passes programmatically although visually it obviously failed.

Shouldn't you update copyright years in the headers of edited files?

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Stanimir Stamenkov
> 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: Complete the assertion condition in BodyInheritedFontSize

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2515/files
  - new: https://git.openjdk.java.net/jdk/pull/2515/files/c7d79b97..112a77b2

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

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 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
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
In reply to this post by Alexander Zuev-3
On Thu, 11 Feb 2021 22:31:21 GMT, Alexander Zuev <[hidden email]> wrote:

> Shouldn't you update copyright years in the headers of edited files?

Sure! The copyright header in `StyleSheet.java` needs updating.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
On Thu, 11 Feb 2021 22:34:28 GMT, Alexey Ivanov <[hidden email]> wrote:

> Shouldn't you update copyright years in the headers of edited files?

I don't know if I should.  I'll do it if I'm told it is necessary.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
On Thu, 11 Feb 2021 22:36:06 GMT, Stanimir Stamenkov <[hidden email]> wrote:

>>> Shouldn't you update copyright years in the headers of edited files?
>>
>> Sure! The copyright header in `StyleSheet.java` needs updating.
>
>> Shouldn't you update copyright years in the headers of edited files?
>
> I don't know if I should.  I'll do it if I'm told it is necessary.

>  Copyright (c) 1997, 2018

Should I update it like: "Copyright (c) 1997, 2021"?  That is, update the second year, or should I add a new one: "Copyright (c) 1997, 2018, 2021"?

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Stanimir Stamenkov
> 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: Update Copyright year of edited StyleSheet.java

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2515/files
  - new: https://git.openjdk.java.net/jdk/pull/2515/files/112a77b2..324ddb17

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

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
Reply | Threaded
Open this post in threaded view
|

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

Stanimir Stamenkov
In reply to this post by Stanimir Stamenkov
> 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: Correct typo: fontSizePrecentage -> fontSizePercentage

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2515/files
  - new: https://git.openjdk.java.net/jdk/pull/2515/files/324ddb17..454a02fa

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

  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 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
Reply | Threaded
Open this post in threaded view
|

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

Alexander Zuev-3
On Thu, 11 Feb 2021 22:52:56 GMT, Stanimir Stamenkov <[hidden email]> 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: Correct typo: fontSizePrecentage -> fontSizePercentage

Marked as reviewed by kizune (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Prasanta Sadhukhan-2
In reply to this post by Stanimir Stamenkov
On Thu, 11 Feb 2021 22:52:56 GMT, Stanimir Stamenkov <[hidden email]> 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: Correct typo: fontSizePrecentage -> fontSizePercentage

test/jdk/javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java line 149:

> 147:
> 148:     public static void main(String[] args) throws Throwable {
> 149:         TestWrongCSSFontSize test = new TestWrongCSSFontSize(

A minor nit...
Actually in all ImageIO.write() we normally pass the file with extension like image.png but here if the argument passed to testcase does not give .png extension, it just stores as that name, which is kind of irritation. I think we can hardcode the filename in call to ImageIO.write to CSSimage.png or something similar, instead of getting the path in main.

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

PR: https://git.openjdk.java.net/jdk/pull/2515
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
On Fri, 12 Feb 2021 11:37:30 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> Stanimir Stamenkov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8260687: Correct typo: fontSizePrecentage -> fontSizePercentage
>
> test/jdk/javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java line 149:
>
>> 147:
>> 148:     public static void main(String[] args) throws Throwable {
>> 149:         TestWrongCSSFontSize test = new TestWrongCSSFontSize(
>
> A minor nit...
> Actually in all ImageIO.write() we normally pass the file with extension like image.png but here if the argument passed to testcase does not give .png extension, it just stores as that name, which is kind of irritation. I think we can hardcode the filename in call to ImageIO.write to CSSimage.png or something similar, instead of getting the path in main.

This is a side effect of my change: one parameter is always passed to the test and now the test always save an image; the test should not save the image by default. To enable saving the image, a second parameter needs to be passed. Hardcoding the name of the image, perhaps based on the first parameter `w3cUnits` seems reasonable.

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

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