-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8359430: Test 'javax/swing/plaf/windows/bug4991587.java' automatically failed on Windows 2025 x64 with message "Failed. Compilation failed: Compilation failed" #25883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back dnguyen! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
Please confirm that you can reproduce JDK-4991587 by the updated test. |
The updated test does contain all of the text within the blue border now as expected (instead of overlapping or being outside the borders of the textRect, as described in the original issue). I have updated the summary to this PR as well. Thanks! |
You could also take this opportunity to automate the test. I believe it should be possible to verify that the border is not distorted. |
f.add(button2); | ||
|
||
return f; | ||
} | ||
|
||
static class MyButtonUI extends WindowsButtonUI { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use composition here: extend BasicButtonUI
, create a WindowsButtonUI
¹ object and forward all the methods² to the instance of WindowsButtonUI
. In the paintText
method, draw the rectangle and then call the implementation of your WindowsButtonUI
.
¹ Use WindowsButtonUI.createUI
to create an instance.
² All the overridden methods in WindowsButtonUI
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good if we could eliminate the reference to WindowsButtonUI. It might also be useful to check the behavior across all L&Fs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good if we could eliminate the reference to WindowsButtonUI.
But we want the regression to reproduce the original problem. The problem was specific to WindowsButtonUI
, in WindowsGraphicsUtils
according to comments in the bug.
It may be impossible to reproduce the problem without referencing WindowsButtonUI
. If it's possible, I'm all for it.
It might also be useful to check the behavior across all L&Fs.
The bug was in Windows L&F, I don't think it's applicable to other L&F.
On Windows, the disabled text was painted with “shadow” with offset of (1, 1) which could be painted over another part of the button. Other L&Fs are free to paint the button differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be impossible to reproduce the problem without referencing WindowsButtonUI. If it's possible, I'm all for it.
That should be checked on old jdk build.
The bug was in Windows L&F, I don't think it's applicable to other L&F.
It depends on how these offsets were used previously, maybe we displayed the text outside the button, which shouldn't happen for all L&Fs.
Did you run the test with Java 5 where JDK-4991587 isn't fixed? I don't expect a border added to a button will reproduce the problem. I may be wrong. |
Compilation error on windows with
WindowsButtonUI
used. Replaced it withBasicButtonUI
to maintain overriding thepaintText
method but this was not the same as Windows L&F buttons. I attempted to importWindowsButtonUI
and extending it, but the class isfinal
. Instead, I chose to add a LineBorder in the same way as when a Rectangle was drawn to keep the same test behavior. It seems the extra 1 pixel width is unnecessary now when testing, so the changes only include removingMyButtonUI
and adding a LineBorder.This updated test now compiles and runs as expected. The test has all of the text within the blue button border. There does not seem to be any overlapping or out of bounds text as originally described with the original test.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25883/head:pull/25883
$ git checkout pull/25883
Update a local copy of the PR:
$ git checkout pull/25883
$ git pull https://git.openjdk.org/jdk.git pull/25883/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25883
View PR using the GUI difftool:
$ git pr show -t 25883
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25883.diff
Using Webrev
Link to Webrev Comment