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