RFR: 8323801: <s> tag doesn't strikethrough the text

[Prasanta Sadhukhan]

On Fri, 9 Feb 2024 14:03:27 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java line 2504:
>> 
>>> 2502:             tagMap.put(HTML.Tag.SPAN, ca);
>>> 2503:             tagMap.put(HTML.Tag.STRIKE, conv);
>>> 2504:             tagMap.put(HTML.Tag.S, conv);
>> 
>> Should we not update the spec
>> https://github.com/openjdk/jdk/blob/71b46c38a820319851cade2a15d64a657d4d7284/src/java.desktop/share/classes/javax/swing/text/html/HTMLDocument.java#L2314-L2319
>> to mention that it is now `ConvertAction `and not `CharacterAction`
>
> It's a good question.
> 
> We can't do it because `ConvertAction` is not public. In fact, `B`, `FONT`, `I`, `STRIKE`, `SUB`, `SUP`, `U` use `ConvertAction` instead of the specified `CharacterAction`.
> 
> The bigger problem is that `ConvertAction` does not extend `CharacterAction`, which means the implementation is different from what is specified. It is the result of [JDK-4171509](https://bugs.openjdk.org/browse/JDK-4171509).
> 
> There are a few more inconsistencies between the table and the real implementation. There are also references to non-public action classes, such as `TitleAction`, `LinkAction`…

Probably we can do away with those tags which mention internal class OR mention the public action instead, like TagAction instead of ConvertAction
HiddenAction instead of TitleAction etc

>> test/jdk/javax/swing/text/html/HTMLDocument/HTMLStrike.java line 84:
>> 
>>> 82:         if (!errors.isEmpty()) {
>>> 83:             errors.delete(errors.length() - 2, errors.length());
>>> 84:             throw new Error(errors + " must have both "
>> 
>> Should it be RuntimeException inline with other tests?
>
> `Error` is shorter and conveys the reason better.
> 
> If you insist, I can use `RuntimeException`.

I guess it's about consistency...majority client tests uses RE while handful uses Error...not sure if there is any consensus on it..

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17659#discussion_r1485803912
PR Review Comment: https://git.openjdk.org/jdk/pull/17659#discussion_r1485804641