On Fri, 11 Jul 2025 18:35:29 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:

>> This PR updates the last (!!) remaining platform-specific part of the 
>> java.desktop module: macos.
>> The unix/windows were previously addressed in: 
>> https://github.com/openjdk/jdk/pull/24170 , 
>> https://github.com/openjdk/jdk/pull/24941 and 
>> https://github.com/openjdk/jdk/pull/25439.
>> 
>> Note: Unlike on other platforms, the initial change for macOS caused the 
>> "BadSerializationTest" to fail. This test relies on internal details of the 
>> Aqua Look & Feel that were previously exposed via serialized data (and 
>> already [fails](https://bugs.openjdk.org/browse/JDK-8277817) on other 
>> platforms). Adding the final modifier to the class changes its 
>> serialVersionUID, which triggered the failure even on macOS.
>> To avoid this issue, I have reverted the changes to those internal classes 
>> for now, see 
>> https://github.com/openjdk/jdk/pull/25607/commits/ff7e64b6e38a1a809d434628f304e49408135697
>> 
>> To download the diff file and filter only the modified lines you can use the 
>> following script:
>> 
>> 
>> curl -s 
>> https://patch-diff.githubusercontent.com/raw/openjdk/jdk/pull/25607.diff -o 
>> d.txt && grep -E '^+|^-' d.txt | grep -vE '^+++|^---'
>> 
>> ====================================================
>> 
>> The `java.desktop` module currently lacks proper use of the `@Override` 
>> annotation for methods and the `final` modifier for classes. While similar 
>> changes were previously made in the 
>> [JavaSound](https://github.com/openjdk/jdk/commit/e0c7d59246cf36644d494eced76e4b9d96ff1ded#diff-ae3e5f9c40fe25ef03e7a89844de174ef5c15e6179d769e2a4bcb7e73688c9b5),
>>  [java.desktop/windows](https://github.com/openjdk/jdk/pull/24170) (and some 
>> of the classes on demand), these changes are not as critical now due to the 
>> new jdk "encapsulation", but they are still useful for improving code 
>> consistency.
>> 
>> The public API was not affected so there is no need to worry about a CSR
>> 
>> Note: I will submit additional patches in this area later because:
>> 
>> 1. Only lines with `@Override` and `final` were modified to keep the diff 
>> clear - header dates were not updated (that could be covered by one patch 
>> similar to [this](https://bugs.openjdk.org/browse/JDK-8345797))
>> 2. I skipped adding `@Override` for anonymous classes
>> 
>> Any feedback or suggestions are welcome!
>> 
>> Here is a 
>> [link](https://patch-diff.githubusercontent.com/raw/openjdk/jdk/pull/25607.diff)
>>  to a simple diff file, it might be more convenient for reviewing the 
>> changes.
>> 
>> The build was successful, and I ran all the open jtreg tests without issues. 
>> But...
>
> Sergey Bylokhov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into over_macos
>  - Fix java/awt/dnd/BadSerializationTest/BadSerializationTest.java test
>  - restore formatting
>  - final in java.desktop/macos
>  - override in java.desktop/macos

Marked as reviewed by azvegint (Reviewer).

src/java.desktop/macosx/classes/com/apple/laf/AquaFonts.java line 38:

> 36: 
> 37: @SuppressWarnings("serial") // JDK implementation class
> 38: public final class AquaFonts {

This final class has two protected methods:


protected static FontUIResource getMiniControlTextFont() {
protected static FontUIResource getSmallControlTextFont() {


There may be more, but I suppose this can be handled as a separate issue.

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

PR Review: https://git.openjdk.org/jdk/pull/25607#pullrequestreview-3017737749
PR Review Comment: https://git.openjdk.org/jdk/pull/25607#discussion_r2205783736

Reply via email to