On Fri, 29 Mar 2024 15:32:12 GMT, Alexey Ivanov <aiva...@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.

src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java line 
3446:

> 3444:             @Override
> 3445:             void convertAttributes(HTML.Tag t, MutableAttributeSet 
> attr) {
> 3446:                 Object newDecoration = 
> attr.getAttribute(CSS.Attribute.TEXT_DECORATION);

parameter 't' isn't used. Why is that ?

test/jdk/javax/swing/text/html/HTMLDocument/HTMLTextDecoration.java line 62:

> 60:             <p><span style='text-decoration: underline'><s>underline + 
> line-through?</s></span></p>
> 61:             <p><span style='text-decoration: underline'><strike>underline 
> + line-through?</strike></span></p>
> 62: 

Suppose there's this HTML
<p><s><span style='text-decoration: line-through'>underline + 
line-through?</span></s></p>


ie a strike through is specified in both ways. Does the merge code handle that 
? I think it probably does but
adding this case to the test might be a good idea.

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

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

Reply via email to