RFR: 8326734: text-decoration applied to <span> lost when mixed with <u> or <s>

Alexey Ivanov aivanov at openjdk.org
Tue Apr 9 20:27:10 UTC 2024


On Tue, 9 Apr 2024 19:10:26 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> The value of the [`text-decoration`](https://www.w3.org/TR/REC-CSS1/#text-decoration) CSS property is not inherited correctly in Swing. If the `<span>` element is mixed with `<u>` or `<s>`, only the value from the `style` attribute of `<span>` is applied.
>> 
>> The fix to this issue is not as simple as that for the previous one in PR #17659, [JDK-8323801](https://bugs.openjdk.org/browse/JDK-8323801). Even in the seemingly simple case where `<u>` is followed by `<span style='text-decoration: line-through'>`, the situation is more complex because the styles are stored in `MuxingAttributeSet` in different elements of the array.
>> 
>> To resolve this problem, `CSS.Attribute.TEXT_DECORATION` is treated as a special case. Indeed, it is a special case: the values set to a single `text-decoration` property should be combined across the entire tree of nested HTML elements and their styles.
>> 
>> So, `MuxingAttributeSet` looks for `text-decoration` in the entire array and combines all the values.
>> 
>> The same way, `StyleSheet` also goes up the inheritance chain by combining the current value of `text-decoration` with that from `getResolveParent`.
>> 
>> The `ConvertSpanAction` combines the value of `text-decoration` of adjacent `<span>` elements.
>> 
>> Finally, `ConvertAction` and `CharacterAction` are refactored. The `ConvertAction` class duplicated the code from `CharacterAction`. Now `ConvertAction` extends `CharacterAction` and overrides a method to provide additional handling.
>> 
>> Thus, [JDK-8325620](https://bugs.openjdk.org/browse/JDK-8325620) is also resolved by this PR, the action used for `<b>`, `<i>`, `<u>` is `CharacterAction` as specified.
>
> test/jdk/javax/swing/text/html/HTMLDocument/HTMLTextDecoration.java line 125:
> 
>> 123:                     contentView.getAttributes()
>> 124:                                .getAttribute(CSS.Attribute.TEXT_DECORATION)
>> 125:                                .toString();
> 
> It might be good to add a null check before calling .toString() since `contentView.getAttributes().getAttribute(CSS.Attribute.TEXT_DECORATION)` can return null.

No, it can't. In the test, all the views that end up in `contentView` must have the `CSS.Attribute.TEXT_DECORATION` attribute and its value must contain both `underline` and `line-through`. It is what the test verifies.

If the test is modified and `contentView` does not have the `CSS.Attribute.TEXT_DECORATION` attribute, the test will fail with `NullPointerException` which is fine.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18550#discussion_r1558240319


More information about the client-libs-dev mailing list