On Thu, 29 Aug 2024 14:11:01 GMT, Oliver Schmidtmer <d...@openjdk.org> wrote:
>> FileSystemJava had no valid implementation for pathFileName since the >> function was renamed from pathGetFileName to pathFileName in the >> FileSystem.h from WebKit > > Oliver Schmidtmer has updated the pull request incrementally with one > additional commit since the last revision: > > add test The change looks good, For reference the related change in Webkit repo is : https://github.com/WebKit/WebKit/commit/3cab8bc8c7d3f8dc0bad8529df249bc09e6402ec Provided a few comments that need to be attended. Verified that test fails without this patch and passes with it. modules/javafx.web/src/main/native/Source/WTF/wtf/java/FileSystemJava.cpp line 276: > 274: } > 275: > 276: String pathFileName(const String& path) This method is used 2 times in file `modules/javafx.web/src/main/native/Source/WebCore/fileapi/FileCocoa.mm`. It needs correction. [[1]](https://github.com/openjdk/jfx/blob/a53bc589ac37d490b1406a2b977097a06bf5ac74/modules/javafx.web/src/main/native/Source/WebCore/fileapi/FileCocoa.mm#L62), [[2]](https://github.com/openjdk/jfx/blob/a53bc589ac37d490b1406a2b977097a06bf5ac74/modules/javafx.web/src/main/native/Source/WebCore/fileapi/FileCocoa.mm#L64) modules/javafx.web/src/test/java/test/javafx/scene/web/FileTest.java line 94: > 92: }); > 93: } > 94: } The test requires a few changes: 1. Correct copyright year 2. Several imports can be removed. 3. method `private State getLoadState()` is unused 4. method `getScriptString()` can be replaced with a string member variable 5. Line 70,71 needs format correction I tried these changes locally, attaching the file with required changes. Please check [FileTest.java.zip](https://github.com/user-attachments/files/16813919/FileTest.java.zip) ------------- Changes requested by arapte (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1551#pullrequestreview-2271545512 PR Review Comment: https://git.openjdk.org/jfx/pull/1551#discussion_r1738181928 PR Review Comment: https://git.openjdk.org/jfx/pull/1551#discussion_r1738182435
