<Swing Dev> RFR: 8231286: HTML font size too large with high-DPI scaling and W3C_LENGTH_UNITS [v7]

[Alexey Ivanov]

On Thu, 4 Feb 2021 11:59:29 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>>> 
>>> 
>>> I have written automatic test for [JDK-8260687](https://bugs.openjdk.java.net/browse/JDK-8260687): Inherited font size is smaller than expected when using StyleSheet to add styles. You can find it in my JDK-8260687 branch:
>>> https://github.com/aivanov-jdk/jdk/commits/JDK-8260687
>>> There's also a commit which modifies the test from [JDK-8257664](https://bugs.openjdk.java.net/browse/JDK-8257664) to run in two modes with and without W3C_LENGTH_UNITS.
>>> 
>>> We have several options:
>>> 
>>>     1. As this PR contains the fix for JDK-8260687, you can cherry-pick the test into your branch.
>>> 
>>>     2. I can create a new JBS issue to add the test for JDK-8260687 separately.
>>> 
>>>     3. You can revert the fix in your PR, and I'll add it to the test and then raise a new PR to resolve JDK-8260687.
>>> 
>>> 
>>> I have no strong preference for any of the options.
>> 
>> My preference will be 
>> 1) if @mperktold can pick the test from https://github.com/aivanov-jdk/jdk/commit/6d0b40db4400dac03567b5cab4ad45b103ea94d0 and add into this PR
>> else 2)
>> @mperktold can you also issue this comment "/issue add JDK-8260687" once @aivanov-jdk approves this PR.
>
>> My preference will be
>> 
>>     1. if @mperktold can pick the test from [aivanov-jdk at 6d0b40d](https://github.com/aivanov-jdk/jdk/commit/6d0b40db4400dac03567b5cab4ad45b103ea94d0) and add into this PR
>>        else 2)
>>        @mperktold can you also issue this comment "/issue add JDK-8260687" once @aivanov-jdk approves this PR.
> 
> Good!
> 
> The only thing is that the test file has three commits now:
> aivanov-jdk at 6d0b40d, aivanov-jdk at b50d650, aivanov-jdk at f9e99777
> The final version of the test file [BodyInheritedFontSize.java](https://github.com/aivanov-jdk/jdk/blob/f9e997776fe478d99f0bbb6739ded10ec79b4caa/test/jdk/javax/swing/text/html/StyleSheet/8260687/BodyInheritedFontSize.java).
> 
> Then this PR would include the fixes for two issues, thus the command
> `/issue add JDK-8260687` 
> would add another resolved bug to the commit.
> 
> Optionally, issuing the following commands:
> `/contributor add psadhukhan`
> `/contributor add  aivanov`
> will add Prasanta and myself as contributors.

> 
> 
> > > It seems Toolkit.getDefaultToolkit().getScreenResolution() can return 93/94/95 instead of 96 in mac/linux in internal mach5 testing systems causing failure in this test Test.java. Probably we need to make the testcase hardcoded to 96
> > 
> > 
> > I guess so.
> > Where does this `Test.java` come from?
> > > I also suggest fixing JDK-8260687 to not use font inherit for W3C_LENGTH_UNIT case
> > > ```
> > > --- a/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> > > +++ b/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java
> > > @@ -2823,7 +2823,7 @@ public class StyleSheet extends StyleContext {
> > >          }
> > > 
> > >          Object doGetAttribute(Object key) {
> > > -            if (key == CSS.Attribute.FONT_SIZE && !isDefined(key)) {
> > > +            if (key == CSS.Attribute.FONT_SIZE && !isDefined(key) && !isW3CLengthUnits()) {
> > >                  // CSS.FontSize represents a specified value and we need
> > >                  // to inherit a computed value so don't resolve percentage
> > >                  // value from parent.
> > > ```
> > 
> > 
> > Does such a fix not bring back the problem with relative units for the case where W3C_LENGTH_UNIT is enabled?
> 
> I guess no as this code is recently introduced by JDK-8257664 and I have verified with this testcase and other jtreg regression tests. But if you have a better fix, then please suggest.

Indeed, it does. Why will it not? The modified [TestWrongCSSFontSize.java](https://github.com/openjdk/jdk/blob/16fdd939a4351519c8bc0ca36154443bd0986854/test/jdk/javax/swing/text/html/StyleSheet/TestWrongCSSFontSize.java) (regression test for [JDK-8257664](https://bugs.openjdk.java.net/browse/JDK-8257664)) fails when W3C_LENGTH_UNIT is enabled with the proposed fix: the font size gets bigger than it should as 150% scale gets applied twice to this fragment:
<h2>Foo</h2>

The test uses `body { font-size: 14 }` rule in its `<style>`. But it fails the same way if this is replaced with the correct units:
        editor.setText(TEXT.replace("$FONT_SIZE",
                                    w3cUnits ? "14px" : "14pt"));

Since this is the case, I suggest reverting the [proposed fix](https://github.com/openjdk/jdk/pull/2256/commits/ce2500ced9cf4007599972590e4924bdc87cc829) for [JDK-8260687](https://bugs.openjdk.java.net/browse/JDK-8260687). The modified test passes successfully in both modes when the proposed fix is reverted.

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

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