On Tue, 8 Feb 2022 11:13:20 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. The fix itself looks good. Suggested few minor changes. Regarding test: Observed that the test does not fail without this fix. Please recheck. Also, I would recommend to add new tests to test the behavior mentioned in JBS, and modify any pre-existing tests if they now become invalid with fix. modules/javafx.web/src/main/native/Source/WebCore/platform/java/DataObjectJava.h line 133: > 131: Vector<String> types; > 132: types.appendRange(m_availMimeTypes.begin(), > m_availMimeTypes.end()); > 133: //returns MIME Types available in clipboard.z Minor: The comment should be the first line in the method, as it was before this. The character z at the end must be unintended. modules/javafx.web/src/main/native/Source/WebCore/platform/java/PasteboardJava.cpp line 240: > 238: // TODO: setURL, setFiles, setData, setHtml (needs URL) > 239: data->setPlainText(jGetPlainText()); > 240: data->setData(DataObjectJava::mimeHTML(),jGetPlainText()); Minor: Please add a space after `,` modules/javafx.web/src/test/java/test/javafx/scene/web/HTMLEditingTest.java line 73: > 71: Clipboard.getSystemClipboard().setContent(content); > 72: > 73: The extra line is not required and would recommend to remove. ------------- Changes requested by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/729
