On Sat, 20 Jun 2026 17:54:20 GMT, Michael Strauß <[email protected]> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains nine additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into 8373452.data.format
>> - Merge branch 'master' into 8373452.data.format
>> - 2026
>> - Merge branch 'master' into 8373452.data.format
>> - Merge branch 'master' into 8373452.data.format
>> - junit
>> - whitespace
>> - javadoc
>> - data format
>
> I've been staring at this code, scratching my head and wondering what is
> going on.
>
> First of all, the global `DataFormat` registry does indeed expose some real
> problems. However, replacing the constructor with an interned
> `DataFormat.of(String...)` does not address the underlying design problem. It
> seems to me that the main question here is not whether `DataFormat` should be
> created with a constructor or a factory method, the main question is what
> `DataFormat` is supposed to mean.
>
> The proposed `DataFormat.of(...)` still requires a global ownership model for
> identifiers. That model is ambiguous for overlapping formats:
>
> DataFormat a = DataFormat.of("text/foo");
> DataFormat b = DataFormat.of("text/foo", "text/bar");
>
>
> There is no universally correct interned result here:
> * returning `a` loses the "text/bar" information
> * returning `b` changes what earlier callers meant by "text/foo"
> * throwing keeps the current failure mode
> * mutating `a` to include "text/bar" would break immutability and hash-based
> collections
>
> So the factory does not actually solve the semantic problem. It also does not
> remove the need to support existing public API, as both
> `DataFormat(String...)` and `DataFormat.lookupMimeType(String)` are already
> public.
>
> The current implementation relies too much on identity and registry lookup.
> For example, `QuantumClipboard` maps native MIME strings back through
> `DataFormat.lookupMimeType(...)`, which makes clipboard behavior depend on
> unrelated application initialization order.
>
> I think we should treat `DataFormat` primarily as an immutable value
> descriptor: `new DataFormat("text/foo", "text/bar")` means that this format
> can be represented by any of the specified identifiers.
>
> Then we define another operation, let's call it `boolean
> DataFormat.matches(String)`. This method returns `true` if any of the
> identifiers is equal to the given string. This is the question that clipboard
> code _actually_ needs to answer. (Note that this operation is not strictly
> required, as the identifiers are already exposed with
> `DataFormat.getIdentifiers()`, so anyone can answer the question that a
> `matches` method would answer.)
>
> `Clipboard.getContentTypes()` should report what the clipboard _actually_
> advertises, not an arbitrary alias group that an application may have
> registered globally. If the native clipboard reports only `text/foo` then
> `getContentTypes()` can reasonably return `Set.of(new
> DataFormat("text/foo"))`.
>
> But this must still work:
>
> clipboard.hasContent(n...
My thoughts are similar to @mstr2's.
First, I'm not confident the implementation of `DataFormat` is compatible with
its use. The class is used in conjunction with `Clipboard`/`Dragboard` as a
marker for the type of data it contains.
Because `DataFormat` is just a thin wrapper of a `Set<String>` (where the
strings are identifiers/mime types), and the content is populate through a
`Map<DataFormat, Object>`, it essentially serves as an intermediary between the
ids and the value. That is, the clipboard is essentially `ids -> value`.
Likewise, the content is retrieved by `DataFormats`, not by the ids they
contain. For example,
var df = new DataFormat("text/a", "text/b");
var cc = new ClipboardContent();
cc.put(df, "value");
clipboard.setContent(cc);
// ... and then
clipboard.getContent(df); // "value"
There is no direct way to ask the clipboard specifically for a `"text/a"`
despite having matching content:
var df2 = new DataFormat("text/a");
clipboard.get(df2); // null
This does not strike me as a correct behavior to begin with. The receiving
application is interested in identifiers, not aggregates of identifiers:
`getContent(String id)`. If the receiving application is interested in
`"text/plain"`, but the publishing application wanted to provide more
information about the text and used `new DataFormat("text/plain", "text/rtf")`,
the content will not be found. As Michael said, a manual lookup for an id in
all data formats is possible.
What a publishing application can do in this case is use 1 `DataFormat` per id:
put both `new DataFormat("text/plain")` and `new DataFormat("text/rtf")` in the
clipboard content with the same value. Indeed, the docs of `Clipboard` say
> The `Clipboard` operates on the concept of having a single conceptual item on
> the clipboard at any one time -- though it may be placed on the clipboard in
> different formats.
and give the example
content.putString("Some text");
content.putHtml("<b>Some</b> text");
so that there are 2 `DataFormat`s, not a single one aggregating the plain and
html ids. But then what purpose does `DataFormat` serve?
As for the multiplicity problem - the clipboard could contain more than one
value per identifier:
var df1 = new DataFormat("text/a", "text/b");
var df2 = new DataFormat("text/a", "text/c");
var cc = new ClipboardContent();
cc.put(df1, "value1");
cc.put(df2, "value2");
clipboard.setContent(cc);
and what should an application interested in `"text/a"` find, `"value1"` or
`"value2"`? Perhaps this is why there can't exist more than 1 association of an
identifier with a `DataFormat` and the global registry is needed (although
`WeakReferenceQueue` seems like a poor choice for it).
My conclusion is that the content should actually be represented as
`Map<String, Object>` where the string is the identifier. This eliminates the
problem of multiplicity by virtue of the `Map` contract: the user can then
decide to solve multiplicity either by "last entry wins" via standard `put` or
by "first entry wins" via `putIfAbsent`.
The threading issue can be eliminated by using a `ConcurrentMap`, but it
shouldn't be about concurrently registering an identifier with a `DataFormat`,
but rather about populating the clipboard concurrently. The synchronization
should be done at a different level, in my opinion.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/2006#issuecomment-4760401792
