On Fri, 25 Feb 2022 07:21:57 GMT, Jay Bhaskar <d...@openjdk.java.net> wrote:

>> Issue: The DataObject uses m_availMimeTypes as Vector of strings, and 
>> appending mime types in pasteboard operation like setPlainText, Hence it 
>> will append duplicate mime types in m_availMimeTypes , in this case the 
>> length clipboardData.types would be wrong, and duplicates data would be 
>> copied to clipboard target.
>> Solution: Use ListHashSet data Structure instead of Vector for 
>> m_availMimeTypes.
>
> Jay Bhaskar has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - formating change
>  - formating change

The fix looks good to me, but the test still needs additional changes. Then new 
test should fail without your fix and pass with your fix (as of now it passes 
either with or without your fix, so it isn't actually testing the fix).

Also, I think you might have misunderstood Ambarish's comment about the test. 
Rather than modifying the existing `clipboardGetDataOnPaste ` test method, I 
recommend to leave it alone and add a new test method.

modules/javafx.web/src/test/java/test/javafx/scene/web/HTMLEditingTest.java 
line 91:

> 89:                     executeScript("srcInput.defaultValue").toString(), 
> defaultText);
> 90:             assertEquals("Source clipboard onpaste data", 
> getEngine().executeScript("srcInput.value").
> 91:                     toString(), clipboardData + defaultText);

The expected and actual values are backwards; the first of the two objects 
being compared should be the expected value. I see that this is a preexisting 
bug in the test. I recommend fixing this in the new test (while leaving the 
existing test method as is).

modules/javafx.web/src/test/java/test/javafx/scene/web/HTMLEditingTest.java 
line 96:

> 94:             assertEquals("Target onpaste data size", getEngine().
> 95:                             
> executeScript("document.getElementById(\"clipboardData\").innerText").toString(),
> 96:                     "2");

Same comment as above about expected and actual being backwards.

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

PR: https://git.openjdk.java.net/jfx/pull/729

Reply via email to