On Tue, 20 Jan 2026 22:01:58 GMT, Kevin Rushforth <[email protected]> wrote:
>> ### Summary
>>
>> This PR makes the `DataFormat` constructor private:
>>
>> private DataFormat(@NamedArg("ids") String... ids)
>>
>>
>> and replaces it with
>>
>> public static DataFormat of(String ... ids)
>>
>>
>> ### Problem
>>
>> There seems to be several issues with DataFormat API and implementation
>> discovered during a Clipboard-related code review:
>>
>> 1. `static DataFormat::lookupMimeType(String)` is not thread safe: while
>> iterating over previously registered entries in the `DATA_FORMAT_LIST`
>> another thread might create a new instance (DataFormat L227)
>>
>> 2. `public DataFormat(String...)` constructor might throw an
>> `IllegalArgumentException` if one of the given mime types is already
>> assigned to another `DataFormat`. The origin of this requirement is unclear,
>> but one possible issue I can see is if the application has two libraries
>> that both attempt to create a `DataFormat` for let's say `"text/css"`. Then,
>> depending on the timing or the exact code path, an exception will be thrown
>> for which the library(-ies) might not be prepared. The constructor is also
>> not thread safe.
>>
>> 3. To avoid a situation mentioned in bullet 2, a developer would is
>> typically call `lookupMimeType()` to obtain an already registered instance,
>> followed by a constructor call if such an instance has not been found. An
>> example of such code can be seen in webkit/UIClientImpl:299 - but even then,
>> despite that two-step process being synchronized, the code might still fail
>> if *some other* library or the application attempts to create a new instance
>> of DataFormat, since the constructor itself is not synchronized.
>>
>> 4. `DataFormat(new String[] { null })` is allowed but makes no sense!
>>
>> Why do we need to have the registry of previously created instances?
>> Unclear. My theory is that the DataFormat allows to have multiple mime-types
>> (ids) - example being `DataFormat.FILES = new
>> DataFormat("application/x-java-file-list", "java.file-list");` - and the
>> registry was added to prevent creation of a `DataFormat` with just one id
>> for some reason.
>>
>> What should be done?
>> - find out why we need this registry in the first place i.e. what could
>> happen if we have multiple DataFormat instances with overlapping ids.
>> - if the registry is needed add a new factory method, something like
>> `DataFormat::of(String ...)` which is properly synchronized. This method
>> will be called by the constructor to retain the backward compatibility.
>> - deprecate (possibly for removal) `DataFormat::lookupMimeType(St...
>
> modules/javafx.graphics/src/main/java/javafx/scene/input/DataFormat.java line
> 133:
>
>> 131: */
>> 132: @Deprecated(since = "27")
>> 133: private DataFormat(@NamedArg("ids") String... ids) {
>
> This is an incompatible change. You will need to go through the usual process
> for such changes where it is deprecated for removal in one release and
> removed in a subsequent release.
should we also print a warning to `stderr`?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2006#discussion_r2710486058
