On Wed, 15 Apr 2026 22:25:46 GMT, Andy Goryachev <[email protected]> wrote:

>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttribute.java
>>  line 158:
>> 
>>> 156:         int h = getClass().hashCode();
>>> 157:         h = 31 * h + name.hashCode();
>>> 158:         h = 31 * h + type.hashCode();
>> 
>> Should you also add the attribute type (isCharacterAttribute(), etc)? It 
>> seems more bullet-proof, although in practice, it seems unlikely that there 
>> would be two attributes with the same name where one was a character attr 
>> and the other was a paragraph attr.
>
> This one is interesting, it actually points to a deeper problem: currently it 
> is possible to create two different `StyleAttribute`s with the same name and 
> different types.  
> 
> They will work fine in hash tables and `StyleAttributeMap`s, but they will 
> fail when marshaling to external storage, for example, by the 
> `RichTextModel`, because it stores/looks up the attributes by name.  
> 
> This poses a number of problems:
> 1. we might need to ensure uniqueness by inventing a static registry.
> 2. even with a registry, we might end up with a compatibility issue down the 
> line.  For example, the application might declare an attribute with the name 
> that will be clobbered by the next version of RichTextArea.
> 3. there another, addmittedly more theoretical, possibility, when the 
> application and let's say a third party library both decide to declare two 
> different attributes with the same name.
> 
> One possible solution is to ensure that system attributes use some special 
> character in their name (!b) and disallow that character in custom 
> attributes.  This will decouple custom attributes from ours.
> 
> What do you think?

This seems worth considering further. I recommend filing a follow-up issue so 
that we don't hold up this PR.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3124746840

Reply via email to