On Tue, 9 Apr 2024 17:27:57 GMT, Phil Race <p...@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 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.

Because you didn't add the backticks <code>`</code> around your sample, it's 
interpreted as HTML, and I can't really see it.

If the both tags have the same value for the `text-decoration` property, it 
works without the fix. I'm sure it works with the fix, however, in some cases 
the value of the property may be `line-through,line-through`.

I'll add another test if you think such a scenario is worth verifying too.

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

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

Reply via email to