Re: RFR: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs
On Fri, 17 May 2024 03:37:21 GMT, Prasanta Sadhukhan wrote: > Inadvertent mention of Netscape in Javadoc is removed.. Marked as reviewed by abhiscxk (Committer). - PR Review: https://git.openjdk.org/jdk/pull/19276#pullrequestreview-2062312829
RFR: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs
Inadvertent mention of Netscape in Javadoc is removed.. - Commit messages: - 8332403: Anachronistic reference to Netscape Communicator in Swing API docs - 8332403: Anachronistic reference to Netscape Communicator in Swing API docs Changes: https://git.openjdk.org/jdk/pull/19276/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19276=00 Issue: https://bugs.openjdk.org/browse/JDK-8332403 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19276.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19276/head:pull/19276 PR: https://git.openjdk.org/jdk/pull/19276
Integrated: 8331746: Create a test to verify that the cmm id is not ignored
On Mon, 6 May 2024 20:51:55 GMT, Sergey Bylokhov wrote: > The new test to cover the https://bugs.openjdk.org/browse/JDK-8326661 and > verify that the cmm id of the icc profile is properly reported. Before > JDK-8321489 we always report 'lcms' as a cmm id. This pull request has now been integrated. Changeset: 7c750fd9 Author:Sergey Bylokhov URL: https://git.openjdk.org/jdk/commit/7c750fd95b83d0a93b0cce681dcfbbae1f220fdd Stats: 69 lines in 1 file changed: 69 ins; 0 del; 0 mod 8331746: Create a test to verify that the cmm id is not ignored Reviewed-by: prr, dmarkov, aivanov - PR: https://git.openjdk.org/jdk/pull/19110
Re: RFR: 8332416: Add more font selection options to Font2DTest [v2]
On Thu, 16 May 2024 19:16:26 GMT, Phil Race wrote: >> Enhance Font2DTest as follows >> >> - Add main menu Radio Button options so that you select the font to use as >> either >> - (1) Font Family + Style (as now) >> - (2) Font Family + Menu of all members of the Family, replacing the Style >> - (3) List of all fontnames - which can still be adjusted by Style if you >> want. >> The default is (1) so nothing looks different except that I updated the UI >> to use Nimbus instead of Metal. >> >> There's new code to gather these ways of referencing the fonts. >> Also changes were needed for the "Save/Load" options to include the new UI >> state and font settings. > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8332416 BTW, it is probably obvious , but maybe 50% of "reviewing" this will be pulling it down and building it locally and trying it out. Very hard to review by code changes alone. - PR Comment: https://git.openjdk.org/jdk/pull/19273#issuecomment-2116378508
Re: RFR: 8332416: Add more font selection options to Font2DTest [v2]
> Enhance Font2DTest as follows > > - Add main menu Radio Button options so that you select the font to use as > either > - (1) Font Family + Style (as now) > - (2) Font Family + Menu of all members of the Family, replacing the Style > - (3) List of all fontnames - which can still be adjusted by Style if you > want. > The default is (1) so nothing looks different except that I updated the UI to > use Nimbus instead of Metal. > > There's new code to gather these ways of referencing the fonts. > Also changes were needed for the "Save/Load" options to include the new UI > state and font settings. Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8332416 - Changes: - all: https://git.openjdk.org/jdk/pull/19273/files - new: https://git.openjdk.org/jdk/pull/19273/files/0fc0e271..9721de23 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19273=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19273=00-01 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19273.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19273/head:pull/19273 PR: https://git.openjdk.org/jdk/pull/19273
RFR: 8332416: Add more font selection options to Font2DTest
Enhance Font2DTest as follows - Add main menu Radio Button options so that you select the font to use as either - (1) Font Family + Style (as now) - (2) Font Family + Menu of all members of the Family, replacing the Style - (3) List of all fontnames - which can still be adjusted by Style if you want. The default is (1) so nothing looks different except that I updated the UI to use Nimbus instead of Metal. There's new code to gather these ways of referencing the fonts. Also changes were needed for the "Save/Load" options to include the new UI state and font settings. - Commit messages: - 8332416 Changes: https://git.openjdk.org/jdk/pull/19273/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19273=00 Issue: https://bugs.openjdk.org/browse/JDK-8332416 Stats: 459 lines in 2 files changed: 407 ins; 12 del; 40 mod Patch: https://git.openjdk.org/jdk/pull/19273.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19273/head:pull/19273 PR: https://git.openjdk.org/jdk/pull/19273
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v7]
On Thu, 16 May 2024 12:23:44 GMT, Maurizio Cimadamore
wrote:
>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting
>> the use of JNI in the following ways:
>>
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is
>> now considered a restricted operation
>>
>> This PR slightly changes the way in which the JDK deals with restricted
>> methods, even for FFM API calls. In Java 22, the single
>> `--enable-native-access` was used both to specify a set of modules for which
>> native access should be allowed *and* to specify whether illegal native
>> access (that is, native access occurring from a module not specified by
>> `--enable-native-access`) should be treated as an error or a warning. More
>> specifically, an error is only issued if the `--enable-native-access flag`
>> is used at least once.
>>
>> Here, a new flag is introduced, namely
>> `illegal-native-access=allow/warn/deny`, which is used to specify what
>> should happen when access to a restricted method and/or functionality is
>> found outside the set of modules specified with `--enable-native-access`.
>> The default policy is `warn`, but users can select `allow` to suppress the
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This
>> aligns the treatment of restricted methods with other mechanisms, such as
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>>
>> Some changes were required in the package-info javadoc for
>> `java.lang.foreign`, to reflect the changes in the command line flags
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add note on --illegal-native-access default value in the launcher help
src/java.base/share/classes/java/lang/System.java line 2023:
> 2021: * @throws NullPointerException if {@code filename} is {@code
> null}
> 2022: * @throws IllegalCallerException If the caller is in a module
> that
> 2023: * does not have native access enabled.
The exception description is fine, just noticed the other exception
descriptions start with a lowercase "if", this one is different.
src/java.base/share/man/java.1 line 587:
> 585: \f[V]deny\f[R]: This mode disables all illegal native access except for
> 586: those modules enabled by the \f[V]--enable-native-access\f[R]
> 587: command-line option.
"This mode disable all illegal native access except for those modules enabled
the --enable-native-access command-line option".
This can be read to mean that modules granted native access with the command
line option is also illegal native access An alternative is to make the second
part of the sentence a new sentence, something like "Only modules enabled by
the --enable-native-access command line option may perform native access.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603878829
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603875920
Re: RFR: 8331746: Create a test to verify that the cmm id is not ignored [v3]
On Thu, 16 May 2024 04:27:25 GMT, Sergey Bylokhov wrote: >> The new test to cover the https://bugs.openjdk.org/browse/JDK-8326661 and >> verify that the cmm id of the icc profile is properly reported. Before >> JDK-8321489 we always report 'lcms' as a cmm id. > > 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 four additional > commits since the last revision: > > - Merge branch 'openjdk:master' into JDK-8331746 > - Update CustomCMMID.java > - Update CustomCMMID.java > - 8331746: Create a test to verify that the cmm id is not ignored Marked as reviewed by dmarkov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19110#pullrequestreview-2061429810
Re: RFR: 8329667: [macos] Issue with JTree related fix for JDK-8317771
On Thu, 16 May 2024 01:28:37 GMT, Alexander Zuev wrote: > Caching children and selected children of the thee on the native level; > Caching all children of a specific parent in CAccessibility to enhance > recursive children selection algorithm; > Removing fix for JDK-8317771 as no longer needed; @savoptik @kumarabhi006 please review - PR Comment: https://git.openjdk.org/jdk/pull/19255#issuecomment-2115636469
Integrated: 8260633: [macos] java/awt/dnd/MouseEventAfterStartDragTest/MouseEventAfterStartDragTest.html test failed
On Tue, 7 May 2024 18:03:23 GMT, Alisen Chung wrote: > Opening closed dnd test > Test is green on all platforms This pull request has now been integrated. Changeset: 6f7ddbec Author:Alisen Chung URL: https://git.openjdk.org/jdk/commit/6f7ddbec7d0bc459d44b6518fe1d982eaba7f37b Stats: 214 lines in 1 file changed: 214 ins; 0 del; 0 mod 8260633: [macos] java/awt/dnd/MouseEventAfterStartDragTest/MouseEventAfterStartDragTest.html test failed Reviewed-by: serb, dnguyen, tr - PR: https://git.openjdk.org/jdk/pull/19128
Re: RFR: 8331746: Create a test to verify that the cmm id is not ignored [v3]
On Thu, 16 May 2024 04:27:25 GMT, Sergey Bylokhov wrote: >> The new test to cover the https://bugs.openjdk.org/browse/JDK-8326661 and >> verify that the cmm id of the icc profile is properly reported. Before >> JDK-8321489 we always report 'lcms' as a cmm id. > > 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 four additional > commits since the last revision: > > - Merge branch 'openjdk:master' into JDK-8331746 > - Update CustomCMMID.java > - Update CustomCMMID.java > - 8331746: Create a test to verify that the cmm id is not ignored Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19110#pullrequestreview-2060639298
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v7]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Add note on --illegal-native-access default value in the launcher help - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/1c45e5d5..3a0db276 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=06 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:55:35 GMT, Jaikiran Pai wrote: >> We already do, see >> https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 > > This is slightly different from what we do in the other PR for unsafe memory > access where we specify the default in the launcher's help text too > https://github.com/openjdk/jdk/pull/19174/files#diff-799093930b698e97b23ead98c6496261af1e2e33ec7aa9261584870cbee8a5eaR219. > > I don't have a strong opinion on this, I think either one is fine. Ah, apologies, I was looking in the wrong place. I agree that we should specify default in the launcher, as well as in the man pages. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603233038
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:47:13 GMT, Maurizio Cimadamore wrote: >> src/java.base/share/classes/sun/launcher/resources/launcher.properties line >> 72: >> >>> 70: \ by code in modules for which native access is not >>> explicitly enabled.\n\ >>> 71: \ is one of "deny", "warn" or "allow".\n\ >>> 72: \ This option will be removed in a future release.\n\ >> >> Should this specify the current default value for this option if it isn't >> set? > > We already do, see > https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 This is slightly different from what we do in the other PR for unsafe memory access where we specify the default in the launcher's help text too https://github.com/openjdk/jdk/pull/19174/files#diff-799093930b698e97b23ead98c6496261af1e2e33ec7aa9261584870cbee8a5eaR219. I don't have a strong opinion on this, I think either one is fine. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603205279
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:42:48 GMT, Jaikiran Pai wrote: > Hello Maurizio, in the current mainline, we have code in `LauncherHelper` > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/launcher/LauncherHelper.java#L636 > where we enable native access to all unnamed modules if an executable jar > with `Enable-Native-Access: ALL-UNNAMED` manifest is being launched. For such > executable jars, what is the expected semantics when the launch also > explicitly has a `--enable-native-access=M1,M2` option. Something like: > > ``` > java --enable-native-access=M1,M2 -jar foo.jar > ``` > > where `foo.jar` has `Enable-Native-Access: ALL-UNNAMED` in its manifest. The options are additive - e.g. the enable-native-access in the manifest will add up to the enable-native-access in the command line, so effectively it will be as if you were running with --enable-native-access=M1,M2,ALL-UNNAMED > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 72: > >> 70: \ by code in modules for which native access is not >> explicitly enabled.\n\ >> 71: \ is one of "deny", "warn" or "allow".\n\ >> 72: \ This option will be removed in a future release.\n\ > > Should this specify the current default value for this option if it isn't set? We already do, see https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2115012361 PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603195671
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Wed, 15 May 2024 16:08:17 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment Hello Maurizio, in the current mainline, we have code in `LauncherHelper` https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/launcher/LauncherHelper.java#L636 where we enable native access to all unnamed modules if an executable jar with `Enable-Native-Access: ALL-UNNAMED` manifest is being launched. For such executable jars, what is the expected semantics when the launch also explicitly has a `--enable-native-access=M1,M2` option. Something like: java --enable-native-access=M1,M2 -jar foo.jar where `foo.jar` has `Enable-Native-Access: ALL-UNNAMED` in its manifest. - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2115005638
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Wed, 15 May 2024 16:08:17 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment src/java.base/share/classes/sun/launcher/resources/launcher.properties line 72: > 70: \ by code in modules for which native access is not > explicitly enabled.\n\ > 71: \ is one of "deny", "warn" or "allow".\n\ > 72: \ This option will be removed in a future release.\n\ Should this specify the current default value for this option if it isn't set? - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603157916
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v5]
On Tue, 26 Mar 2024 08:22:52 GMT, Prasanta Sadhukhan wrote: >> @aivanov-jdk >> >>> This looks weird… So you're saying Label[Enabled].textForeground and >>> Label[Disabled].textForeground are used for Nimbus (and Synth and GTK) >>> instead of Label.foreground and Label.disabledForeground which are used for >>> other L >> >> As per my understanding, Yes, for Nimbus LAF the UI properties are different >> than other LAF. >> >>> Shouldn't we fix the problem by correcting the keys instead? It looks like >>> it's what you're doing for specific components. >> >> I am not sure if it is a problem or nimbus LAF is supposed to be like this. >> >>> Is it specified anywhere that Synth-based L use different constants? It >>> results in incorrect colors. >> >> Need to check on this. >> >>> If a developer sets the common properties, should they override >>> Look-and-Feel defaults? >> >> Will check and revert back. > >> @aivanov-jdk >> >> > This looks weird… So you're saying Label[Enabled].textForeground and >> > Label[Disabled].textForeground are used for Nimbus (and Synth and GTK) >> > instead of Label.foreground and Label.disabledForeground which are used >> > for other L >> >> As per my understanding, Yes, for Nimbus LAF the UI properties are different >> than other LAF. >> >> > Shouldn't we fix the problem by correcting the keys instead? It looks like >> > it's what you're doing for specific components. >> >> I am not sure if it is a problem or nimbus LAF is supposed to be like this. >> >> > Is it specified anywhere that Synth-based L use different constants? It >> > results in incorrect colors. >> >> Need to check on this. >> >> > If a developer sets the common properties, should they override >> > Look-and-Feel defaults? >> >> Will check and revert back. > > > As I understood, what `Label.background` color to be set, is determined via > **`Nimbus.Overrides`** property > i.e., if `Label.background` is set to `Nimbus.Overrides`, it should > override the `label.setBackground()` user color setting > else > if `Nimbus.Overrides` is not set or set to null, then `Label.setBackground()` > will hold precedent even if `Label.background` property is set > > > Similarly, if `Nimbus.Overrides` is set with `Label.background` color **AND** > Label[Enabled].background is also set, > then > Label[Enabled].background will have priority over > Label.background which will have priority over > Label.setBackground() > > but this is also controlled by another boolean property > **`Nimbus.Overrides.InheritDefaults`** in way that the above precedence is > valid only if if Nimbus.Overrides.InheritDefaults is true > > if **`Nimbus.Overrides.InheritDefaults`** is false, then > Label[Enabled].background will be ignored > and > Label.background will have priority over > Label.setBackground() > > This is somewhat explained in > `https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/javax/swing/plaf/nimbus/package-summary.html` > and tested in` test/jdk/javax/swing/plaf/nimbus/ColorCustomizationTest.java` > so in a way, it seems widget[state].color setting is applicable for Nimbus > only since we dont have GTK.Overrides property @prsadhuk I have updated the PR to skip the GTK L for both the test bug as we discussed that GTK L may not support the color setting by UIManager. As per the documentation for UIManager https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/javax/swing/UIManager.html, it states that `The set of defaults a particular look and feel supports is defined and documented by that look and feel. In addition, each look and feel, or ComponentUI provided by a look and feel, may access the defaults at different times in their life cycle. Some look and feels may aggressively look up defaults, so that changing a default may not have an effect after installing the look and feel. Other look and feels may lazily access defaults so that a change to the defaults may effect an existing look and feel. Finally, other look and feels might not configure themselves from the defaults table in any way. None-the-less it is usually the case that a look and feel expects certain defaults, so that in ge neral a ComponentUI provided by one look and feel will not work with another look and feel`. So, it seems the UIManager color setting may not be honored in GTK L Please have a look on the updated changes. - PR Comment: https://git.openjdk.org/jdk/pull/17763#issuecomment-2114569799
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v6]
> JLabel text is not painted with the LAF defined foreground color in GTK LAF. > In GTK LAF the foreground color is retrieved by using native system APIs. Fix > is to return the foreground color if it is set by LAF defined property > otherwise return the default color by calling native APIs. > Applet based test has been converted to automatic test and check for all > installed LAFs. CI testing is green for test suite and individual test. Link > attached in JBS. Abhishek Kumar 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 seven additional commits since the last revision: - Revert back the product fix and update test to skip GTK L - Merge - separate method to get LAF defined color - extended fix for disabled checkbox and radiobutton - Headful tag revert back and condition check for nimbus - jtreg tag headful removed - JLable foreground color fix for GTK LAF - Changes: - all: https://git.openjdk.org/jdk/pull/17763/files - new: https://git.openjdk.org/jdk/pull/17763/files/360e21e8..56268d41 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17763=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17763=04-05 Stats: 1432245 lines in 12034 files changed: 312659 ins; 679637 del; 439949 mod Patch: https://git.openjdk.org/jdk/pull/17763.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17763/head:pull/17763 PR: https://git.openjdk.org/jdk/pull/17763
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Wed, 15 May 2024 13:32:38 GMT, Magnus Ihse Bursie wrote:
> Hi Julian, sorry for not getting back to you.
>
> The problem from my PoV is that this is a very large and intrusive patch in
> the build of the actual product, for a claimed problem in the tangential
> hsdis library. I have not understood a pressing business need to get a
> Windows/gcc port for hsdis, which means I can't really prioritize trying to
> understand this patch.
>
> As you know, the build system does not currently really separate between "the
> OS is Windows" and "the toolchain is Microsoft". I understand that you want
> to fix that, and in theory I support it. However, you must also realize that
> making a complete fix of this requires a lot of work, for something that
> there is no clearly defined need. (After all, cl.exe works fine to create the
> build, has no apparent issues, and is as far as an "official" compiler for
> Windows as you can get.) That makes it fall squarely in the WIBNIs bucked
> ("wouldn't it be nice if...").
>
> If you can fix just the hsdis build without changing anything outside the
> hsdis Makefiles, that would be a different story. Such a change would be
> limited in scope, easy to say it will not affect the product proper, and be
> easier to review.
Oh, no worries. Sorry for sounding a little impatient.
The problem is that there are areas in the build system that will need changing
to support hsdis compilation, not just the hsdis Makefile and autoconf file
itself. I can try to work on minimizing the amount of changes to non-hsdis
files (I was hoping the current changes were small enough, but it seems they
are not), but I believe it's impossible to achieve this by only touching the
hsdis Makefile and lib-hsdis.m4. That is, there will necessarily be changes, no
matter how minimal, to non-hsdis files.
As for why do this at all, I was mainly driven by seeing the hack in place for
the binutils backend on Windows. It only supports Cygwin, of which the gcc
installation is subpar compared to the newer gcc provided by some MSYS2
subsystems (MINGW64 gcc is primarily battle tested on MSYS2, on Cygwin it
doesn't get as much attention and misses out further fixes and enhancements
from the MSYS2 team, for instance it even links to the obsolete msvcrt
library), and the hack itself has several flaws that don't seem apparent at
first (For instance, -lpthread will link to libwinpthread.dll and result in an
invisible dependency on that dll depending on the Windows platform, which will
cause loading hsdis to silently fail depending on how it's loaded for seemingly
random reasons!). I wanted to replace that (or I guess provide a better
alternative) by adding semi proper support to the hsdis build for the more
modern and better battle tested gcc that some MSYS2 subsystems provide, to
streamline comp
iling the binutils hsdis on Windows. So this is mainly about hsdis-binutils on
Windows. hsdis-capstone I also decided to support because it's small and
relatively easy to pile on top of the existing work, and MSYS2 also provides
Capstone as well. The same unfortunately could not be said for hsdis-llvm,
which was significantly more complex than Capstone is
I'm not sure where to go from here. I could support gcc for hsdis binutils by
extending the hack that exists for Cygwin, but I _really_ don't want to do
that. I could enhance the build system to semi properly support gcc for hsdis
only, as I've done, but the changes will necessarily touch non hsdis files as
well, no matter how minimal they are. I'll return this to draft for the time
being I suppose, I'd be interested in hearing which way forward you prefer
though
But while I work on making this change even smaller and easier to review, could
I ask the above questions again? (Well, except for the FIXPATH one, you can
ignore that)
> @magicus I think I might need some help here. Currently all the Cygwin stuff
> is gated behind ifeq ($(TOOLCHAIN_TYPE), microsoft) checks... should they be
> behind isBuildOsEnv cygwin checks instead? I don't know how to add support
> for Cygwin gcc for most of hsdis, since it is quite different from the more
> modern gcc distributions that are found in places like MSYS2 and MINGW64. But
> most of the existing logic seems to accomodate more for the Microsoft
> compiler than it is concerned about the OS environment being used, and for
> this reason I can't tell which of the 2 checks to use for the existing hack
> that switches from microsoft to gcc. Also, gcc doesn't require FIXPATH, but
> Microsoft does, but I don't want to check for microsoft inside
> TOOLCHAIN_FIND_COMPILER, what should I do instead to ensure gcc doesn't get
> FIXPATH while Microsoft does?
> Also @magicus, what is the typical path passed to --with-binutils like on
> Windows? Something like
> --with-binutils=/c/Users/vertig0/Downloads/binutils-2.42 doesn't work
> correctly, since the include path to dis-asm.h
