RFR: 8337481: File API: file.name contains path instead of name [v2]
Ambarish Rapte
arapte at openjdk.org
Fri Aug 30 08:26:27 UTC 2024
On Thu, 29 Aug 2024 14:11:01 GMT, Oliver Schmidtmer <duke at 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
More information about the openjfx-dev
mailing list