Integrated: 8271923: [macos] the text color on the selected disabled tabbed pane button remains white making text unreadable
On Mon, 23 Aug 2021 12:32:20 GMT, Prasanta Sadhukhan wrote: > It is seen that if a JTabbedPane is unfocused, it's title is painted with > **white** text on grey background > as opposed to **black** text on grey background in unfoucsed native app on > macOSX Catalina > and is somewhat not legible. > This can be seen with SwingSet2 demo with InternalFrame or JTabbedPane demo > and any native app, making focus toggle between the two. > > Issue was TabbedPane always draw with "selectedTabTitleNormalColor" which is > white. Although Aqua L&F defined selectedTabTitleDisabledColor but it is not > used as TabbedPane does not check if focus is there in current frame and draw > accordingly, which native app does. > > Proposed fix is to check for frame is active or not and draw text color > accordingly. > Since it is not affecting BigSur (where even if native app active or not text > is always drawn in same color), it is only restricted to Catalina and lower. This pull request has now been integrated. Changeset: df05b4d1 Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/df05b4d1a13e55755107695ad9ea7a8a1084901a Stats: 9 lines in 2 files changed: 8 ins; 0 del; 1 mod 8271923: [macos] the text color on the selected disabled tabbed pane button remains white making text unreadable Reviewed-by: jdv - PR: https://git.openjdk.java.net/jdk/pull/5217
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v2]
On Sat, 4 Sep 2021 11:06:14 GMT, Prasanta Sadhukhan wrote: >> You need to override the paintIcon method in the InvertableImageIcon >> returned by the AquaImageFactory.getMenuArrowIcon() or you can create `class >> MenuArrowIcon extends InvertableImageIcon,` and override it there. >> >> Also take a look to another usage of InvertableImageIcon for >> "MenuItemCheckIcon", should we disable it as well? > > MenuItemCheckIcon disable-ness is also solved along with this fix. > JCheckBoxMenuItem component for MenuItemCheckIcon test is added additionally. Any more comments on this? - PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8271923: [macos] the text color on the selected disabled tabbed pane button remains white making text unreadable [v2]
On Mon, 6 Sep 2021 08:00:36 GMT, Prasanta Sadhukhan wrote: >> It is seen that if a JTabbedPane is unfocused, it's title is painted with >> **white** text on grey background >> as opposed to **black** text on grey background in unfoucsed native app on >> macOSX Catalina >> and is somewhat not legible. >> This can be seen with SwingSet2 demo with InternalFrame or JTabbedPane demo >> and any native app, making focus toggle between the two. >> >> Issue was TabbedPane always draw with "selectedTabTitleNormalColor" which is >> white. Although Aqua L&F defined selectedTabTitleDisabledColor but it is not >> used as TabbedPane does not check if focus is there in current frame and >> draw accordingly, which native app does. >> >> Proposed fix is to check for frame is active or not and draw text color >> accordingly. >> Since it is not affecting BigSur (where even if native app active or not >> text is always drawn in same color), it is only restricted to Catalina and >> lower. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Use NonFocus color It will be difficult to prove the issue for text color as robot will pick the pixels for tabbedpane along with background, not only text. Also, it's not for all macos, it's only for Catalina...we don't have requires tag to omit specific macos.. - PR: https://git.openjdk.java.net/jdk/pull/5217
Integrated: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'."
On Wed, 11 Aug 2021 09:03:57 GMT, Prasanta Sadhukhan wrote: > The test fails in CI testing citing expected "rededited" but found > "redEDITED", which seems to point to fact that either there is a CAPS_LOCK > key switched on or some other test did not use SHIFT key corectly ie, > pressed but not released. > > Considering many more tests would have failed if CAPS_LOCK is turned on, I > tried to find if some tests uses SHIFT key mistakenly and it seems > JRadioButton test has press/release order wrong and it uses SHIFT. Rectified > that. > > In addition, corrected some known CI issues ie waiting after frame is made > visible, made frame to centre. Along with that, also modified testcase to > check lowercase string. Since the original JDK-4275046 issue is about newly > entered text is made part of cell or not, it does not matter if it's in > lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, > this test will not be affected. > > CI run for several iterations running these 2 tests one after another in all > platforms is green. This pull request has now been integrated. Changeset: cec6c068 Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/cec6c068b03d890312e50b448fbc26102c635249 Stats: 21 lines in 5 files changed: 12 ins; 7 del; 2 mod 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." 8257540: javax/swing/JFileChooser/8041694/bug8041694.java failed with "RuntimeException: The selected directory name is not the expected 'd ' but 'D '." Reviewed-by: serb - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v2]
On Fri, 3 Sep 2021 05:54:45 GMT, Sergey Bylokhov wrote: >> I will like to know how because as it is pointed out, paintArrow delegates >> drawing to ImageIcon#paintIcon which is in shared code and this is mac >> specific issue so it needs to be handled before we call ImageIcon#paintIcon > > You need to override the paintIcon method in the InvertableImageIcon returned > by the AquaImageFactory.getMenuArrowIcon() or you can create `class > MenuArrowIcon extends InvertableImageIcon,` and override it there. > > Also take a look to another usage of InvertableImageIcon for > "MenuItemCheckIcon", should we disable it as well? MenuItemCheckIcon disable-ness is also solved along with this fix. JCheckBoxMenuItem component for MenuItemCheckIcon test is added additionally. - PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v5]
> It is seen in macos disabled JMenuItem arrow is not disabled even though > JMenuItem itself is disabled. > In native app, same menuitem arrow is disabled for disabled menuitem. > > Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a > ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate > MultiResolutionCachedImage via getResolutionVariant() by calling > AquaUtils#generateFilteredImage. > It does not take into account if disabled arrow icon image needs to be drawn > or not, so it is always enabled. > > Proposed fix is to generate a disabled ImageIcon image of the same arrow icon > and use it for disabled state. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Add menuitem checkicon test - Changes: - all: https://git.openjdk.java.net/jdk/pull/5310/files - new: https://git.openjdk.java.net/jdk/pull/5310/files/d2db8808..3c2f69fa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=03-04 Stats: 13 lines in 1 file changed: 10 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5310.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5310/head:pull/5310 PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v4]
> It is seen in macos disabled JMenuItem arrow is not disabled even though > JMenuItem itself is disabled. > In native app, same menuitem arrow is disabled for disabled menuitem. > > Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a > ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate > MultiResolutionCachedImage via getResolutionVariant() by calling > AquaUtils#generateFilteredImage. > It does not take into account if disabled arrow icon image needs to be drawn > or not, so it is always enabled. > > Proposed fix is to generate a disabled ImageIcon image of the same arrow icon > and use it for disabled state. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Override paintIcon - Override paintIcon - Changes: - all: https://git.openjdk.java.net/jdk/pull/5310/files - new: https://git.openjdk.java.net/jdk/pull/5310/files/63b591cc..d2db8808 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=02-03 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5310.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5310/head:pull/5310 PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v3]
> It is seen in macos disabled JMenuItem arrow is not disabled even though > JMenuItem itself is disabled. > In native app, same menuitem arrow is disabled for disabled menuitem. > > Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a > ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate > MultiResolutionCachedImage via getResolutionVariant() by calling > AquaUtils#generateFilteredImage. > It does not take into account if disabled arrow icon image needs to be drawn > or not, so it is always enabled. > > Proposed fix is to generate a disabled ImageIcon image of the same arrow icon > and use it for disabled state. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Override paintIcon - Changes: - all: https://git.openjdk.java.net/jdk/pull/5310/files - new: https://git.openjdk.java.net/jdk/pull/5310/files/ba8f30b5..63b591cc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=01-02 Stats: 24 lines in 2 files changed: 15 ins; 8 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5310.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5310/head:pull/5310 PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v2]
On Thu, 2 Sep 2021 18:10:55 GMT, Sergey Bylokhov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Cache icon
>
> src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java line 419:
>
>> 417: } else {
>> 418: arrowIcon.paintIcon(c, g, arrowIconRect.x,
>> arrowIconRect.y);
>> 419: }
>
> It will be even better to implement it in the same way as done in other L&fs
> like windows/metal.
I will like to know how because as it is pointed out, paintArrow delegates
drawing to ImageIcon#paintIcon which is in shared code and this is mac specific
issue so it needs to be handled before we call ImageIcon#paintIcon
-
PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled [v2]
> It is seen in macos disabled JMenuItem arrow is not disabled even though > JMenuItem itself is disabled. > In native app, same menuitem arrow is disabled for disabled menuitem. > > Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a > ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate > MultiResolutionCachedImage via getResolutionVariant() by calling > AquaUtils#generateFilteredImage. > It does not take into account if disabled arrow icon image needs to be drawn > or not, so it is always enabled. > > Proposed fix is to generate a disabled ImageIcon image of the same arrow icon > and use it for disabled state. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Cache icon - Changes: - all: https://git.openjdk.java.net/jdk/pull/5310/files - new: https://git.openjdk.java.net/jdk/pull/5310/files/c3510ba1..ba8f30b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=00-01 Stats: 16 lines in 1 file changed: 10 ins; 4 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5310.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5310/head:pull/5310 PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
On Thu, 2 Sep 2021 10:48:40 GMT, Sergey Bylokhov wrote:
>> That is for MenuArrowIcon which is also there for Metal and Windows L&F in
>> addition with MenuItemArrowIcon(). Aqua does not have it for MenuItem.
>> The control for aqua menuitem goes via ImageIcon#paintIcon for menuitem
>> arrow which does not have enabled/disabled parameter so it was done the way
>> it is now.
>
>> That is for MenuArrowIcon which is also there for Metal and Windows L&F in
>> addition with MenuItemArrowIcon(). Aqua does not have it for MenuItem.
>
> But this icon is used for the menuitem as well, isn't it? You can add this
> line to the test and check:
>
>
> UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel");
> + UIManager.getDefaults().put("Menu.arrowIcon", "");
>
>
>> The control for aqua menuitem goes via ImageIcon#paintIcon for menuitem
>> arrow which does not have enabled/disabled parameter so it was done the way
>> it is now.
>
> I guess it is go via Icon.paintIcon() at AquaMenuPainter:408 , this is
> similar to metal and windows L&Fs. The first parameter of paintIcon is a
> component, so all information can be read there. And depending on the state
> that icon can draws default arrow, or creates/caches the disabled icon.
>
>
> That is for MenuArrowIcon which is also there for Metal and Windows L&F in
> addition with MenuItemArrowIcon(). Aqua
> And depending on the state that icon can draws default arrow, or
> creates/caches the disabled icon.
I guess that is what it is being done, no? Instead of inside paintArrow()
method, I am creating the disabled icon outside.
-
PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
On Thu, 2 Sep 2021 09:26:16 GMT, Sergey Bylokhov wrote: >> In other L&Fs, it creates their own icon >> for ex, MetalLookAndFeel >> "MenuItem.arrowIcon",(LazyValue) t -> >> MetalIconFactory.getMenuItemArrowIcon(), >> and WindowsLookAndFeel >> "MenuItem.arrowIcon", menuItemArrowIcon, >> >> but for AquaL&F it does not create it's own factory icon and rely on >> BasicIconFactory$MenuItemArrowIcon, so tweaking aqua arrow icon inline with >> how other do will cause design change, so I think this fix caters to the >> problem in view of this aqua limitation. > >> but for AquaL&F it does not create it's own factory icon and rely on >> BasicIconFactory$MenuItemArrowIcon, so tweaking aqua arrow icon inline with >> how other do will cause design change, so I think this fix caters to the >> problem in view of this aqua limitation. > > Are you sure that the Aqua does not have its own factory? the > BasicIconFactory$MenuItemArrowIcon is an empty class. > AquaLookAndFeel.java: > "Menu.arrowIcon",(LazyValue) t -> AquaImageFactory.getMenuArrowIcon(), That is for MenuArrowIcon which is also there for Metal and Windows L&F in addition with MenuItemArrowIcon(). Aqua does not have it for MenuItem. The control for aqua menuitem goes via ImageIcon#paintIcon for menuitem arrow which does not have enabled/disabled parameter so it was done the way it is now. - PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
On Thu, 2 Sep 2021 04:46:48 GMT, Sergey Bylokhov wrote: >> In MenuArrowIcon, the arrow icon is drawn there itself using drawPolygon but >> in Aqua, it is drawn via imageicon image so it will be a change in design >> and will make it more complex and it might introduce bug in hidpi/retina. I >> will like to keep it simple following the current design in aqua. > > On windows it is painted via skin, I guess in GTK as well. But it really does > not matter how it is drawn, the important part is that the icon itself knows > how to draw proper appearance. In other L&Fs, it creates their own icon for ex, MetalLookAndFeel "MenuItem.arrowIcon",(LazyValue) t -> MetalIconFactory.getMenuItemArrowIcon(), and WindowsLookAndFeel "MenuItem.arrowIcon", menuItemArrowIcon, but for AquaL&F it does not create it's own factory icon and rely on BasicIconFactory$MenuItemArrowIcon, so tweaking aqua arrow icon inline with how other do will cause design change, so I think this fix caters to the problem in view of this aqua limitation. - PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
On Wed, 1 Sep 2021 23:57:07 GMT, Sergey Bylokhov wrote: >> It is seen in macos disabled JMenuItem arrow is not disabled even though >> JMenuItem itself is disabled. >> In native app, same menuitem arrow is disabled for disabled menuitem. >> >> Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a >> ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate >> MultiResolutionCachedImage via getResolutionVariant() by calling >> AquaUtils#generateFilteredImage. >> It does not take into account if disabled arrow icon image needs to be drawn >> or not, so it is always enabled. >> >> Proposed fix is to generate a disabled ImageIcon image of the same arrow >> icon and use it for disabled state. > > src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java line 301: > >> 299: arrowIcon = new ImageIconUIResource(GrayFilter. >> 300: >> createDisabledImage(((ImageIcon)arrowIcon).getImage())); >> 301: } > > Maybe we do not need to duplicate LookAndFeel.getDisabledIcon() here and > create a new disabled icon on each call for each menu item? > What about tweak the aqua arrow icon, so it will paint itself correctly for > enabled/disabled states. Similar to how the MenuArrowIcon from the > WindowsLookAndFeel and MetalLookAndFeel works. In MenuArrowIcon, the arrow icon is drawn there itself using drawPolygon but in Aqua, it is drawn via imageicon image so it will be a change in design and will make it more complex and it might introduce bug in hidpi/retina. I will like to keep it simple following the current design in aqua. - PR: https://git.openjdk.java.net/jdk/pull/5310
Re: RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
On Tue, 31 Aug 2021 22:35:10 GMT, Sergey Bylokhov wrote: > > > > In native app, same menuitem arrow is disabled for disabled menuitem. > > How it will work if the "apple.laf.useScreenMenuBar" property is set, will > the arrow look the same(disabled) as after this fix? Yes, it is disabled. - PR: https://git.openjdk.java.net/jdk/pull/5310
RFR: 8268084: [macos] Disabled JMenuItem arrow is not disabled
It is seen in macos disabled JMenuItem arrow is not disabled even though JMenuItem itself is disabled. In native app, same menuitem arrow is disabled for disabled menuitem. Issue is when AquaMenuPainter#paintMenuItem() is called, it tries to draw a ImageIcon image of the arrow via ImageIcon#paintIcon which tries to generate MultiResolutionCachedImage via getResolutionVariant() by calling AquaUtils#generateFilteredImage. It does not take into account if disabled arrow icon image needs to be drawn or not, so it is always enabled. Proposed fix is to generate a disabled ImageIcon image of the same arrow icon and use it for disabled state. - Commit messages: - 8268084: [macos] Disabled JMenuItem arrow is not disabled Changes: https://git.openjdk.java.net/jdk/pull/5310/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5310&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268084 Stats: 167 lines in 2 files changed: 166 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5310.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5310/head:pull/5310 PR: https://git.openjdk.java.net/jdk/pull/5310
Integrated: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
On Thu, 19 Aug 2021 06:13:39 GMT, Prasanta Sadhukhan
wrote:
> This is another attempt of fixing JDK-8249674 which created a regression
> JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties for "Tree.leafIcon",
> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>
> The regression which was "when a JTree node is selected, the text should be
> painted using the selected text color (white), matching the color of the
> expansion control. Instead, it is painted using the standard color (black)"
> is also fixed by this fix, although I could not find any mention of this
> behaviour in the spec that selected text color should match the expanded icon
> color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint
> textForeground for cell label, it calls
>
> g.setColor(context.getStyle().getColor(context,
> ColorType.TEXT_FOREGROUND));
>
>
> which then calls SynthStyle#getColor where even though cell renderer
> correctly gets the foreground color, it gets overridden by getColorForState()
> because the color is a UIResource and there's no corresponding color defined
> for that state for Tree.CellRenderer so it uses default "black" color for
> "text" as defined in skin.laf
>
> else if (type == ColorType.TEXT_FOREGROUND) {
> color = c.getForeground();
> }
> }
>
> if (color == null || color instanceof UIResource) {
> // Then use what we've locally defined
> color = getColorForState(context, type);
> }
>
>
> Proposed fix is to check if current foregroundColor for
> DefaultTreeCellRenderer is UIResouce, then use that color itself.
This pull request has now been integrated.
Changeset: e43a907f
Author:Prasanta Sadhukhan
URL:
https://git.openjdk.java.net/jdk/commit/e43a907f20f99fa1f788fcbe2e99631dd01578d4
Stats: 279 lines in 5 files changed: 271 ins; 0 del; 8 mod
8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
Reviewed-by: aivanov
-
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes [v4]
> This is another attempt of fixing JDK-8249674 which created a regression
> JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties for "Tree.leafIcon",
> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>
> The regression which was "when a JTree node is selected, the text should be
> painted using the selected text color (white), matching the color of the
> expansion control. Instead, it is painted using the standard color (black)"
> is also fixed by this fix, although I could not find any mention of this
> behaviour in the spec that selected text color should match the expanded icon
> color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint
> textForeground for cell label, it calls
>
> g.setColor(context.getStyle().getColor(context,
> ColorType.TEXT_FOREGROUND));
>
>
> which then calls SynthStyle#getColor where even though cell renderer
> correctly gets the foreground color, it gets overridden by getColorForState()
> because the color is a UIResource and there's no corresponding color defined
> for that state for Tree.CellRenderer so it uses default "black" color for
> "text" as defined in skin.laf
>
> else if (type == ColorType.TEXT_FOREGROUND) {
> color = c.getForeground();
> }
> }
>
> if (color == null || color instanceof UIResource) {
> // Then use what we've locally defined
> color = getColorForState(context, type);
> }
>
>
> Proposed fix is to check if current foregroundColor for
> DefaultTreeCellRenderer is UIResouce, then use that color itself.
Prasanta Sadhukhan has updated the pull request incrementally with one
additional commit since the last revision:
Add test
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5178/files
- new: https://git.openjdk.java.net/jdk/pull/5178/files/7be3b9b9..95b9fb64
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=02-03
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.java.net/jdk/pull/5178.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5178/head:pull/5178
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes [v3]
> This is another attempt of fixing JDK-8249674 which created a regression
> JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties for "Tree.leafIcon",
> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>
> The regression which was "when a JTree node is selected, the text should be
> painted using the selected text color (white), matching the color of the
> expansion control. Instead, it is painted using the standard color (black)"
> is also fixed by this fix, although I could not find any mention of this
> behaviour in the spec that selected text color should match the expanded icon
> color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint
> textForeground for cell label, it calls
>
> g.setColor(context.getStyle().getColor(context,
> ColorType.TEXT_FOREGROUND));
>
>
> which then calls SynthStyle#getColor where even though cell renderer
> correctly gets the foreground color, it gets overridden by getColorForState()
> because the color is a UIResource and there's no corresponding color defined
> for that state for Tree.CellRenderer so it uses default "black" color for
> "text" as defined in skin.laf
>
> else if (type == ColorType.TEXT_FOREGROUND) {
> color = c.getForeground();
> }
> }
>
> if (color == null || color instanceof UIResource) {
> // Then use what we've locally defined
> color = getColorForState(context, type);
> }
>
>
> Proposed fix is to check if current foregroundColor for
> DefaultTreeCellRenderer is UIResouce, then use that color itself.
Prasanta Sadhukhan has updated the pull request incrementally with one
additional commit since the last revision:
Add test
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5178/files
- new: https://git.openjdk.java.net/jdk/pull/5178/files/fb56b957..7be3b9b9
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=01-02
Stats: 161 lines in 1 file changed: 161 ins; 0 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/5178.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5178/head:pull/5178
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes [v2]
On Wed, 25 Aug 2021 04:11:55 GMT, Prasanta Sadhukhan
wrote:
>> This is another attempt of fixing JDK-8249674 which created a regression
>> JDK-8266510 for which it was backed out.
>> This fix reinstates the UIResource properties for "Tree.leafIcon",
>> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
>> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
>> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>>
>> The regression which was "when a JTree node is selected, the text should be
>> painted using the selected text color (white), matching the color of the
>> expansion control. Instead, it is painted using the standard color (black)"
>> is also fixed by this fix, although I could not find any mention of this
>> behaviour in the spec that selected text color should match the expanded
>> icon color
>> but it's a long standing behaviour, so catered to it.
>> It was happening because SynthLabelUI#paint when it tries to paint
>> textForeground for cell label, it calls
>>
>> g.setColor(context.getStyle().getColor(context,
>> ColorType.TEXT_FOREGROUND));
>>
>>
>> which then calls SynthStyle#getColor where even though cell renderer
>> correctly gets the foreground color, it gets overridden by
>> getColorForState() because the color is a UIResource and there's no
>> corresponding color defined for that state for Tree.CellRenderer so it uses
>> default "black" color for "text" as defined in skin.laf
>>
>> else if (type == ColorType.TEXT_FOREGROUND) {
>> color = c.getForeground();
>> }
>> }
>>
>> if (color == null || color instanceof UIResource) {
>> // Then use what we've locally defined
>> color = getColorForState(context, type);
>> }
>>
>>
>> Proposed fix is to check if current foregroundColor for
>> DefaultTreeCellRenderer is UIResouce, then use that color itself.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Test update
It is reproducible but since is only color we tend to miss it since there is no
rule that it has to be of that color...As I told, I can create manual test if
that is what is needed..
-
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes [v2]
> This is another attempt of fixing JDK-8249674 which created a regression
> JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties for "Tree.leafIcon",
> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>
> The regression which was "when a JTree node is selected, the text should be
> painted using the selected text color (white), matching the color of the
> expansion control. Instead, it is painted using the standard color (black)"
> is also fixed by this fix, although I could not find any mention of this
> behaviour in the spec that selected text color should match the expanded icon
> color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint
> textForeground for cell label, it calls
>
> g.setColor(context.getStyle().getColor(context,
> ColorType.TEXT_FOREGROUND));
>
>
> which then calls SynthStyle#getColor where even though cell renderer
> correctly gets the foreground color, it gets overridden by getColorForState()
> because the color is a UIResource and there's no corresponding color defined
> for that state for Tree.CellRenderer so it uses default "black" color for
> "text" as defined in skin.laf
>
> else if (type == ColorType.TEXT_FOREGROUND) {
> color = c.getForeground();
> }
> }
>
> if (color == null || color instanceof UIResource) {
> // Then use what we've locally defined
> color = getColorForState(context, type);
> }
>
>
> Proposed fix is to check if current foregroundColor for
> DefaultTreeCellRenderer is UIResouce, then use that color itself.
Prasanta Sadhukhan has updated the pull request incrementally with one
additional commit since the last revision:
Test update
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5178/files
- new: https://git.openjdk.java.net/jdk/pull/5178/files/ae916628..fb56b957
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=00-01
Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod
Patch: https://git.openjdk.java.net/jdk/pull/5178.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5178/head:pull/5178
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
On Thu, 19 Aug 2021 06:13:39 GMT, Prasanta Sadhukhan
wrote:
> This is another attempt of fixing JDK-8249674 which created a regression
> JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties for "Tree.leafIcon",
> "Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
> "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
> "Tree.selectionBorderColor" for NimbusLookAndFeel.
>
> The regression which was "when a JTree node is selected, the text should be
> painted using the selected text color (white), matching the color of the
> expansion control. Instead, it is painted using the standard color (black)"
> is also fixed by this fix, although I could not find any mention of this
> behaviour in the spec that selected text color should match the expanded icon
> color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint
> textForeground for cell label, it calls
>
> g.setColor(context.getStyle().getColor(context,
> ColorType.TEXT_FOREGROUND));
>
>
> which then calls SynthStyle#getColor where even though cell renderer
> correctly gets the foreground color, it gets overridden by getColorForState()
> because the color is a UIResource and there's no corresponding color defined
> for that state for Tree.CellRenderer so it uses default "black" color for
> "text" as defined in skin.laf
>
> else if (type == ColorType.TEXT_FOREGROUND) {
> color = c.getForeground();
> }
> }
>
> if (color == null || color instanceof UIResource) {
> // Then use what we've locally defined
> color = getColorForState(context, type);
> }
>
>
> Proposed fix is to check if current foregroundColor for
> DefaultTreeCellRenderer is UIResouce, then use that color itself.
I tried creating automated test for that but not able to..I can create manual
test but sInce it is easily seen in SwingSet2 JTree demo, we can verify that
using SwingSet2..
-
PR: https://git.openjdk.java.net/jdk/pull/5178
Integrated: 5015261: NPE may be thrown if JDesktopIcon is set to null on a JInternalFrame
On Wed, 4 Aug 2021 09:02:25 GMT, Prasanta Sadhukhan wrote: > JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. This pull request has now been integrated. Changeset: 22ef4f06 Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/22ef4f065315c1238216849ce9ce71b8207b43f8 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod 5015261: NPE may be thrown if JDesktopIcon is set to null on a JInternalFrame Reviewed-by: serb, prr, aivanov - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 5015261: NPE may be thrown if JDesktopIcon is set to null on a JInternalFrame [v7]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Fix typo - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/213b2351..7883ce2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
RFR: 8271923: [macos] the text color on the selected disabled tabbed pane button remains white making text unreadable
It is seen that if a JTabbedPane is unfocused, it's title is painted with **white** text on grey background as opposed to **black** text on grey background in unfoucsed native app on macOSX Catalina and is somewhat not legible. This can be seen with SwingSet2 demo with InternalFrame or JTabbedPane demo and any native app, making focus toggle between the two. Issue was TabbedPane always draw with "selectedTabTitleNormalColor" which is white. Although Aqua L&F defined selectedTabTitleDisabledColor but it is not used as TabbedPane does not check if focus is there in current frame and draw accordingly, which native app does. Proposed fix is to check for frame is active or not and draw text color accordingly. Since it is not affecting BigSur (where even if native app active or not text is always drawn in same color), it is only restricted to Catalina and lower. - Commit messages: - Fix - Fix Changes: https://git.openjdk.java.net/jdk/pull/5217/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5217&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271923 Stats: 7 lines in 2 files changed: 4 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5217.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5217/head:pull/5217 PR: https://git.openjdk.java.net/jdk/pull/5217
Re: RFR: 5015261: JInternalFrame causes NPE [v6]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Field spec wording corrected - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/1351b93c..213b2351 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v2]
On Thu, 19 Aug 2021 17:10:27 GMT, Alexey Ivanov wrote: > As an experiment, is it possible to log the current state of modifier keys, > i.e. Shift, Control, Alt, before the key events are sent to edited table > cell? This would confirm our guess whether Shift key is left pressed by a > test which ran before. It seems Toolkit.getLockingKetState() can only be called for CAPS_LOCK, NUM_LOCK, SCROLL_LOCK etc and not for modifiers. So, modifier keys can only be obtained from InputEvent. I used my own event listener inside the MultiResolutionSplashTest to get shift key via isSHiftDown() and run whole java_awt test in CI and it is shown to be "false" in that test so it seems no shift key is pressed at least for this run. -configuration:(4/107)-- Boot Layer add modules: java.desktop add exports: java.desktop/sun.java2d ALL-UNNAMED --System.out:(3/102)-- VK_CAPS_LOCK key state: false VK_SHIFT pressed key state: false VK_SHIFT released key state: false --System.err:(1/16)-- STATUS:Passed. --rerun:(48/5388)*-- Maybe someother thing or some other timing issues happened the time these test fails. - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 5015261: JInternalFrame causes NPE [v5]
On Thu, 19 Aug 2021 16:48:46 GMT, Prasanta Sadhukhan wrote: >> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be >> called with null value. There are many places in JInternalFrame where >> getDesktopIcon() is accessed without null check which might cause NPE. >> Added null check for those cases. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Throw NPE upfront in setter Please review CSR https://bugs.openjdk.java.net/browse/JDK-8272748 - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 5015261: JInternalFrame causes NPE [v5]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Throw NPE upfront in setter - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/3e1a444d..1351b93c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=03-04 Stats: 8 lines in 1 file changed: 4 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272148: JDesktopPane:getComponentCount() returns one extra than expected with GTKLookAndFeel
On Mon, 16 Aug 2021 20:48:45 GMT, Pankaj Bansal wrote: > The "InternalFrame.useTaskBar" is set true in case of GTKL&F only, What if we set this property to false and only set under some condition for GTK L&F, what sort of problem we can expect? Does any jtreg/jck test gets affected if we set it to false? - PR: https://git.openjdk.java.net/jdk/pull/5121
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v2]
On Tue, 17 Aug 2021 12:35:02 GMT, Alexey Ivanov wrote: > > > > Since FIRST failure is > > java/awt/SplashScreen/MultiResolutionSplash/MultiResolutionSplashTest.java > > I tried to find tests that uses VK_SHIFT key which are … > > Can you reproduce the failure locally if the tests are run in the same order? Yes and no. These tests fail in my local system but not with same error message. It is known that some CI test failures is difficult to reproduce locally due to system configuration and environment configuration(via KVM) in CI is quite different from local personal laptop so timing matters. Also, it seems that these failures are intermittent. I waited for few days but it has not happened since, so it 's difficult to pinpoint what test timing issue is causing that. Also, I admit that the test changes are masking this timing issues, but the changes are not wrong. If the original test author had used toLowerCase() initially, I don't think anyone would have objected as the test just checks for new characters without any distinction between lowercase/uppercase and this change would prevent this swing tests from failing. - PR: https://git.openjdk.java.net/jdk/pull/5079
RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
This is another attempt of fixing JDK-8249674 which created a regression
JDK-8266510 for which it was backed out.
This fix reinstates the UIResource properties for "Tree.leafIcon",
"Tree.closedIcon", "Tree.openIcon", "Tree.selectionForeground",
"Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground",
"Tree.selectionBorderColor" for NimbusLookAndFeel.
The regression which was "when a JTree node is selected, the text should be
painted using the selected text color (white), matching the color of the
expansion control. Instead, it is painted using the standard color (black)" is
also fixed by this fix, although I could not find any mention of this behaviour
in the spec that selected text color should match the expanded icon color
but it's a long standing behaviour, so catered to it.
It was happening because SynthLabelUI#paint when it tries to paint
textForeground for cell label, it calls
g.setColor(context.getStyle().getColor(context,
ColorType.TEXT_FOREGROUND));
which then calls SynthStyle#getColor where even though cell renderer correctly
gets the foreground color, it gets overridden by getColorForState() because
the color is a UIResource and there's no corresponding color defined for that
state for Tree.CellRenderer so it uses default "black" color for "text" as
defined in skin.laf
else if (type == ColorType.TEXT_FOREGROUND) {
color = c.getForeground();
}
}
if (color == null || color instanceof UIResource) {
// Then use what we've locally defined
color = getColorForState(context, type);
}
Proposed fix is to check if current foregroundColor for DefaultTreeCellRenderer
is UIResouce, then use that color itself.
-
Commit messages:
- 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
- 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes
Changes: https://git.openjdk.java.net/jdk/pull/5178/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5178&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8271315
Stats: 118 lines in 4 files changed: 110 ins; 0 del; 8 mod
Patch: https://git.openjdk.java.net/jdk/pull/5178.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5178/head:pull/5178
PR: https://git.openjdk.java.net/jdk/pull/5178
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v6]
On Tue, 17 Aug 2021 06:09:51 GMT, Alexander Zuev wrote: >> Initial implementation and a test case. >> >> The problem is that Aqua LaF shows the focused component with the glow on >> the border, hence when the border is not painted the foxus is not displayed. >> The idea is to paint the glowing border on the focused component anyways. > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Restore Graphics color before leaving paintFocus() Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Tue, 17 Aug 2021 06:04:39 GMT, Alexander Zuev wrote: >> You can trace the usage of graphics.getColor() for example in >> AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color >> property is usually saved and then restored. > >> You can trace the usage of graphics.getColor() for example in >> AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color >> property is usually saved and then restored. > > A few lines above paintFocus() is called there is a section that sets color > to something different in case of opaque button and that does not affect the > next step - text painting - since text painter will grab text color and set > the graphics draw color to it. However in the interest of moving forward i > will ad saving the color - after al it is not a performance critical task. Have you checked if button does not have any text? It might affect the drawRoundRect values which are hardcoded now...In MetalButtonUI#paintFocus, it seems they cater to Button focus ring with and without text by taking care of setBounds(). Do we need to do something similar here? - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: RFR: 5015261: JInternalFrame causes NPE [v3]
On Mon, 16 Aug 2021 21:10:10 GMT, Phil Race wrote: >>> We should either prevent null being set or catch all those nulls. >> >> I did that in 1st iteration of this >> PR..https://github.com/openjdk/jdk/pull/4989/files/2a2ab50ded3305f3a9e0f01e90417b484a0b2db4 >> >> >> @prrace Should I bring that fix back? >> >>> Just add advisory doc on the protected field too. >> >>> ok, this is part of what this change do, probably the text should be >>> rephrased there? >> What should be the advisory phrase? Will the text I have added in the >> protected field doc not suffice? > >> > We should either prevent null being set or catch all those nulls. >> >> I did that in 1st iteration of this >> PR..https://github.com/openjdk/jdk/pull/4989/files/2a2ab50ded3305f3a9e0f01e90417b484a0b2db4 >> >> @prrace Should I bring that fix back? >> > > You are reading my "there are two options text" and the code you propose to > bring back is NOT the option > I recommended. > >> > Just add advisory doc on the protected field too. >> >> > ok, this is part of what this change do, probably the text should be >> > rephrased there? >> > What should be the advisory phrase? Will the text I have added in the >> > protected field doc not suffice? > > Something like "subclassers must ensure this is set to a non-null value > during construction and not set this to null" @prrace Any comment on this? I am just preventing "null being set" as was recommended. - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v3]
On Tue, 17 Aug 2021 12:27:35 GMT, Alexey Ivanov wrote: > And using robot to release keys from EDT as part of the clean-up doesn't seem > right. I am just rectifying the order of the key release..It was pressed in order ALT SHIFT CONTROL and the release was ALT SHIFT CONTROL.. which I rectified to be release in order CONTROL SHIFT ALT - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v2]
On Mon, 16 Aug 2021 13:45:49 GMT, Prasanta Sadhukhan wrote: >> The test fails in CI testing citing expected "rededited" but found >> "redEDITED", which seems to point to fact that either there is a CAPS_LOCK >> key switched on or some other test did not use SHIFT key corectly ie, >> pressed but not released. >> >> Considering many more tests would have failed if CAPS_LOCK is turned on, I >> tried to find if some tests uses SHIFT key mistakenly and it seems >> JRadioButton test has press/release order wrong and it uses SHIFT. Rectified >> that. >> >> In addition, corrected some known CI issues ie waiting after frame is made >> visible, made frame to centre. Along with that, also modified testcase to >> check lowercase string. Since the original JDK-4275046 issue is about newly >> entered text is made part of cell or not, it does not matter if it's in >> lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens >> again, this test will not be affected. >> >> CI run for several iterations running these 2 tests one after another in all >> platforms is green. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Testfix for inadvertent CAPS_LOCK enable problem Since FIRST failure is java/awt/SplashScreen/MultiResolutionSplash/MultiResolutionSplashTest.java I tried to find tests that uses VK_SHIFT key which are java/awt/Debug/DumpOnKey/DumpOnKey.java java/awt/Dialog/NestedDialogs/Modal/NestedModalDialogTest.java java/awt/Dialog/NestedDialogs/Modeless/NestedModelessDialogTest.java java/awt/dnd/RecognizedActionTest/RecognizedActionTest.java java/awt/event/KeyEvent/8020209/bug8020209.java java/awt/event/KeyEvent/ExtendedModifiersTest/ExtendedModifiersTest.java java/awt/event/KeyEvent/KeyMaskTest/KeyMaskTest.java java/awt/event/KeyEvent/SwallowKeyEvents/SwallowKeyEvents.java java/awt/event/MouseEvent/MouseButtonsAndKeyMasksTest/MouseButtonsAndKeyMasksTest.java java/awt/FileDialog/MacOSGoToFolderCrash.java java/awt/Focus/8073453/AWTFocusTransitionTest.java java/awt/Focus/8073453/SwingFocusTransitionTest.java java/awt/Focus/FocusTraversalPolicy/ButtonGroupLayoutTraversal/ButtonGroupLayoutTraversalTest.java java/awt/FullScreen/AltTabCrashTest/AltTabCrashTest.java java/awt/keyboard/AllKeyCode/AllKeyCode.java java/awt/List/ActionEventTest/ActionEventTest.java java/awt/Mouse/MouseModifiersUnitTest/MouseModifiersUnitTest_Extra.java java/awt/Mouse/MouseModifiersUnitTest/MouseModifiersUnitTest_Standard.java java/awt/Robot/ModifierRobotKey/ModifierRobotEnhancedKeyTest.java java/awt/Robot/ModifierRobotKey/ModifierRobotKeyTest.java and CAPS_LOCK keys java/awt/keyboard/AllKeyCode/AllKeyCode.java java/awt/Toolkit/Headless/HeadlessToolkit.java java/awt/Toolkit/LockingKeyStateTest/LockingKeyStateTest.java I could not find any misuse of VK_SHIFT apart from in test/jdk/java/awt/List/ActionEventTest/ActionEventTest.java test/jdk/java/awt/dnd/RecognizedActionTest/RecognizedActionTest.java which I rectified. For CAPS_LOCK, LockingKeyState was already rectified as I mentioned in previous comment and HeadlessToolkit is headless which will not affect headful tests - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v3]
> The test fails in CI testing citing expected "rededited" but found > "redEDITED", which seems to point to fact that either there is a CAPS_LOCK > key switched on or some other test did not use SHIFT key corectly ie, > pressed but not released. > > Considering many more tests would have failed if CAPS_LOCK is turned on, I > tried to find if some tests uses SHIFT key mistakenly and it seems > JRadioButton test has press/release order wrong and it uses SHIFT. Rectified > that. > > In addition, corrected some known CI issues ie waiting after frame is made > visible, made frame to centre. Along with that, also modified testcase to > check lowercase string. Since the original JDK-4275046 issue is about newly > entered text is made part of cell or not, it does not matter if it's in > lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, > this test will not be affected. > > CI run for several iterations running these 2 tests one after another in all > platforms is green. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: VK_SHIFT test anomaly fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/5079/files - new: https://git.openjdk.java.net/jdk/pull/5079/files/dd910c58..1de7e162 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5079&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5079&range=01-02 Stats: 14 lines in 2 files changed: 8 ins; 6 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5079.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5079/head:pull/5079 PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 5015261: JInternalFrame causes NPE [v4]
On Tue, 17 Aug 2021 05:09:10 GMT, Sergey Bylokhov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Prevent null being set
>> - Prevent null being set
>
> src/java.desktop/share/classes/javax/swing/JInternalFrame.java line 1321:
>
>> 1319: desktopIcon = d;
>> 1320: firePropertyChange("desktopIcon", oldValue, d);
>> 1321: }
>
> Probably NPE is better in this case? Either way the specification should say
> something about the new behaviour.
Are you saying to throw NPE upfront here?
Regarding the specification, we are just preventing null as per
recommendation..How should it be documented (that we are preventing null from
being thrown later on)?
-
PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 5015261: JInternalFrame causes NPE [v4]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Prevent null being set - Prevent null being set - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/f69c24a1..3e1a444d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=02-03 Stats: 8 lines in 1 file changed: 2 ins; 1 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'." [v2]
> The test fails in CI testing citing expected "rededited" but found > "redEDITED", which seems to point to fact that either there is a CAPS_LOCK > key switched on or some other test did not use SHIFT key corectly ie, > pressed but not released. > > Considering many more tests would have failed if CAPS_LOCK is turned on, I > tried to find if some tests uses SHIFT key mistakenly and it seems > JRadioButton test has press/release order wrong and it uses SHIFT. Rectified > that. > > In addition, corrected some known CI issues ie waiting after frame is made > visible, made frame to centre. Along with that, also modified testcase to > check lowercase string. Since the original JDK-4275046 issue is about newly > entered text is made part of cell or not, it does not matter if it's in > lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, > this test will not be affected. > > CI run for several iterations running these 2 tests one after another in all > platforms is green. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Testfix for inadvertent CAPS_LOCK enable problem - Changes: - all: https://git.openjdk.java.net/jdk/pull/5079/files - new: https://git.openjdk.java.net/jdk/pull/5079/files/404b86ab..dd910c58 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5079&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5079&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5079.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5079/head:pull/5079 PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'."
On Wed, 11 Aug 2021 09:03:57 GMT, Prasanta Sadhukhan wrote: > The test fails in CI testing citing expected "rededited" but found > "redEDITED", which seems to point to fact that either there is a CAPS_LOCK > key switched on or some other test did not use SHIFT key corectly ie, > pressed but not released. > > Considering many more tests would have failed if CAPS_LOCK is turned on, I > tried to find if some tests uses SHIFT key mistakenly and it seems > JRadioButton test has press/release order wrong and it uses SHIFT. Rectified > that. > > In addition, corrected some known CI issues ie waiting after frame is made > visible, made frame to centre. Along with that, also modified testcase to > check lowercase string. Since the original JDK-4275046 issue is about newly > entered text is made part of cell or not, it does not matter if it's in > lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, > this test will not be affected. > > CI run for several iterations running these 2 tests one after another in all > platforms is green. There was one fix done way back regarding almost same tests being failed due to CAPS_LOCK not turned off by test https://github.com/openjdk/jdk/commit/51837b8a13d24279b8f9a1e36dee7320901a9cc9 but I could not find any other test doing something similar with setLockingState... One was in java/awt/Toolkit/Headless/HeadlessToolkit.java but it is marked headless so would not affect headful test so I was thinking it's more of VK_SHIFT issue - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 5015261: JInternalFrame causes NPE [v3]
On Sat, 14 Aug 2021 18:50:58 GMT, Phil Race wrote: > We should either prevent null being set or catch all those nulls. I did that in 1st iteration of this PR..https://github.com/openjdk/jdk/pull/4989/files/2a2ab50ded3305f3a9e0f01e90417b484a0b2db4 @prrace Should I bring that fix back? > Just add advisory doc on the protected field too. > ok, this is part of what this change do, probably the text should be > rephrased there? What should be the advisory phrase? Will the text I have added in the protected field doc not suffice? - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'."
On Sat, 14 Aug 2021 19:14:06 GMT, Phil Race wrote: > am curious how Prasanta decided > test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java might be the problem As I told in initial explanation, I thought it will be either CAPS_LOCK key switched on or some other test did not use SHIFT key corectly ie, pressed but not released. I could not do anything about if someone press CAPS_LOCK so I search for VK_SHIFT being used in all javax/swing reg test and I found this test bug8033699.java is having a wrong order. I am not saying this was the root cause but it needs rectifying, I presume.. Also, the fix I proposed will not have any impact on this test testing 4275046 fix because of the reason mentioned above (original bug is about new characters being part of field, lowercsae/uppercase does not matter) - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
On Fri, 13 Aug 2021 06:18:49 GMT, Alexander Zuev wrote:
>> Initial implementation and a test case.
>>
>> The problem is that Aqua LaF shows the focused component with the glow on
>> the border, hence when the border is not painted the foxus is not displayed.
>> The idea is to paint the glowing border on the focused component anyways.
>
> Alexander Zuev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright year
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 338:
> 336:
> 337: }
> 338: Color ringColor =
> UIManager.getLookAndFeelDefaults().getColor("Focus.color");
I guess we normally call like this in Basic L&F which is extended by different
L&Fs so that it will pick up the defaults from the particular L&F in question,
otherwise UIManager.getColor(). should suffice as Focus.color is defined in
AquaLookAndFeel.
But I am not sure with this hardcoded values..Can't we leverage viewRect or
textRect to get the required coordinates?
-
PR: https://git.openjdk.java.net/jdk/pull/5082
Integrated: 8058704: Nimbus does not honor JTextPane background color
On Thu, 29 Jul 2021 11:44:31 GMT, Prasanta Sadhukhan wrote: > The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. This pull request has now been integrated. Changeset: 0c4be76f Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/0c4be76f7ff3cd5e32c8eb4e04e6986d5f33fbee Stats: 114 lines in 2 files changed: 112 ins; 0 del; 2 mod 8058704: Nimbus does not honor JTextPane background color 6789980: JEditorPane background color not honored with Nimbus L&F Reviewed-by: aivanov, serb - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 5015261: JInternalFrame causes NPE [v2]
On Thu, 12 Aug 2021 22:31:08 GMT, Sergey Bylokhov wrote: >> I guess that is why we are putting up this statement, no so that user thinks >> twice before making it null? Should we make the field private? > > We cannot make it private, we can only update the spec there as well. done - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 5015261: JInternalFrame causes NPE [v3]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Spec clarification - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/0c96a6e3..f69c24a1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=01-02 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'."
On Wed, 11 Aug 2021 09:03:57 GMT, Prasanta Sadhukhan wrote: > The test fails in CI testing citing expected "rededited" but found > "redEDITED", which seems to point to fact that either there is a CAPS_LOCK > key switched on or some other test did not use SHIFT key corectly ie, > pressed but not released. > > Considering many more tests would have failed if CAPS_LOCK is turned on, I > tried to find if some tests uses SHIFT key mistakenly and it seems > JRadioButton test has press/release order wrong and it uses SHIFT. Rectified > that. > > In addition, corrected some known CI issues ie waiting after frame is made > visible, made frame to centre. Along with that, also modified testcase to > check lowercase string. Since the original JDK-4275046 issue is about newly > entered text is made part of cell or not, it does not matter if it's in > lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, > this test will not be affected. > > CI run for several iterations running these 2 tests one after another in all > platforms is green. I am not sure of the actual reason...But I thought physically accessing the system at this time and pressing CAPS will be less possible at this time of pandemic than some test doing something wrong.. In Anycase, I have taken care of that in the test by checking lowercase so that if CAPS or shift is pressed by anyone/anytest, it willnot affect this test (and as I told original bug is about new characters being part of field, lowercsae/uppercase does not matter) - PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 5015261: JInternalFrame causes NPE [v2]
On Thu, 12 Aug 2021 02:20:51 GMT, Sergey Bylokhov wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Remove test >> - Update javadoc clarification > > src/java.desktop/share/classes/javax/swing/JInternalFrame.java line 1308: > >> 1306: * Sets the JDesktopIcon associated with this >> 1307: * JInternalFrame. >> 1308: * Setting null icon is unsupported and may throw NPE >> if used. > > The field itself is protected and can be changed by the user. I guess that is why we are putting up this statement, no so that user thinks twice before making it null? Should we make the field private? - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 5015261: JInternalFrame causes NPE [v2]
On Mon, 9 Aug 2021 04:21:08 GMT, Prasanta Sadhukhan wrote: >> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be >> called with null value. There are many places in JInternalFrame where >> getDesktopIcon() is accessed without null check which might cause NPE. >> Added null check for those cases. > > Prasanta Sadhukhan has updated the pull request incrementally with two > additional commits since the last revision: > > - Remove test > - Update javadoc clarification Any more comment on the modification? - PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Wed, 11 Aug 2021 11:22:50 GMT, Alexander Zuev wrote:
> Initial implementation and a test case.
>
> The problem is that Aqua LaF shows the focused component with the glow on the
> border, hence when the border is not painted the foxus is not displayed. The
> idea is to paint the glowing border on the focused component anyways.
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 333:
> 331: final Border border = b.getBorder();
> 332: if (border instanceof AquaButtonBorder) {
> 333: ((AquaButtonBorder)border).paintButton(b, g, 0, 0,
> b.getWidth(), b.getHeight());
Does having 0,0 not create a problem in multiscreen environment?
-
PR: https://git.openjdk.java.net/jdk/pull/5082
RFR: 8272232: javax/swing/JTable/4275046/bug4275046.java failed with "Expected value in the cell: 'rededited' but found 'redEDITED'."
The test fails in CI testing citing expected "rededited" but found "redEDITED", which seems to point to fact that either there is a CAPS_LOCK key switched on or some other test did not use SHIFT key corectly ie, pressed but not released. Considering many more tests would have failed if CAPS_LOCK is turned on, I tried to find if some tests uses SHIFT key mistakenly and it seems JRadioButton test has press/release order wrong and it uses SHIFT. Rectified that. In addition, corrected some known CI issues ie waiting after frame is made visible, made frame to centre. Along with that, also modified testcase to check lowercase string. Since the original JDK-4275046 issue is about newly entered text is made part of cell or not, it does not matter if it's in lowercase/uppercase and if such CAPS_LOCK or Shift key problem happens again, this test will not be affected. CI run for several iterations running these 2 tests one after another in all platforms is green. - Commit messages: - 8272232: javax/swing/JTable/4275046/bug4275046.java failed with Expected value in the cell: 'rededited' but found 'redEDITED'. Changes: https://git.openjdk.java.net/jdk/pull/5079/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5079&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272232 Stats: 5 lines in 2 files changed: 4 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5079.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5079/head:pull/5079 PR: https://git.openjdk.java.net/jdk/pull/5079
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v6]
> The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Update test - Changes: - all: https://git.openjdk.java.net/jdk/pull/4930/files - new: https://git.openjdk.java.net/jdk/pull/4930/files/31676791..45f3f4c5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=04-05 Stats: 64 lines in 1 file changed: 14 ins; 34 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v4]
On Mon, 9 Aug 2021 15:08:13 GMT, Alexey Ivanov wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Fix >> - Merge test > > test/jdk/javax/swing/plaf/nimbus/TestNimbusBGColor.java line 25: > >> 23: /* >> 24: * @test >> 25: * @bug 8058704 > > I believe both bugs should be listed. Done - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8272123: Problem list 4 jtreg tests which regularly fail on macos-aach64 [v2]
On Mon, 9 Aug 2021 15:06:55 GMT, Phil Race wrote: >> As per the bug comments, these tests fail too often on macOS arm. >> This will be too noisy for CI. So I would like to problem list them for >> mac-arm > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8272123: Problem list 4 jtreg tests which regularly fail on macos-aarch64 Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5044
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v5]
> The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Optimize test - Changes: - all: https://git.openjdk.java.net/jdk/pull/4930/files - new: https://git.openjdk.java.net/jdk/pull/4930/files/8e01458d..31676791 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=03-04 Stats: 17 lines in 1 file changed: 7 ins; 10 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v4]
> The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Fix - Merge test - Changes: - all: https://git.openjdk.java.net/jdk/pull/4930/files - new: https://git.openjdk.java.net/jdk/pull/4930/files/f308e231..8e01458d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=02-03 Stats: 309 lines in 3 files changed: 135 ins; 174 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Mon, 9 Aug 2021 13:01:17 GMT, Alexey Ivanov wrote: > > > > > The tests are nearly the same: the only difference is the component being > > > tested. I suggest merging the two tests into one test which tests both > > > components one after another. > > > > > > I would rather have as separate test as the fix is in 2 different > > component, I could have raised separate PRs as fix area is different > > although fix seems to be same..and I don't see it as anyhting other than > > test optimization.. > > If you absolutely cannot approve without test merged, then please let me > > know. I would try to do > > I'm for fixing these two issues under one PR. The fix is basically the same, > so it makes it easier to review and track changes later. > > Yes, it's kind of a test optimisation. Yet the tests are similar in nature, > they exercise the same behaviour in two different components. It just makes > sense to re-use the code. If a fix is necessary to one test, it is necessary > for the other — same idea why avoiding duplicate code is good. > > It's not a show-stopper though if everyone agrees it's better to have two > different tests. OK..Not a problem. Merged... - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Wed, 4 Aug 2021 21:23:48 GMT, Alexey Ivanov wrote: > > > The tests are nearly the same: the only difference is the component being > tested. I suggest merging the two tests into one test which tests both > components one after another. I would rather have as separate test as the fix is in 2 different component, I could have raised separate PRs as fix area is different although fix seems to be same..and I don't see it as anyhting other than test optimization.. If you absolutely cannot approve without test merged, then please let me know. I would try to do - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v3]
On Thu, 5 Aug 2021 08:37:07 GMT, Prasanta Sadhukhan wrote: >> The Nimbus look and feel ignores the configured background color of a >> JTextPane and always uses white. >> Every other look and feel tested (Metal, Motif, and Windows) correctly >> honors the configured background color of a JTextPane. >> Issue seems to be in the hardcoded background color in nimbus skin which is >> rectified via the fix. >> After this fix, the background color is correctly updated to whatever is >> being set via JTextPane.setBackground(). >> >> CI all test run is green. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Use background componentPropertyName All CI test including JCK are green. - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8272123: Problem list 4 jtreg tests which regularly fail on macos-aach64
On Mon, 9 Aug 2021 04:28:28 GMT, Sergey Bylokhov wrote: >> As per the bug comments, these tests fail too often on macOS arm. >> This will be too noisy for CI. So I would like to problem list them for >> mac-arm > > test/jdk/ProblemList.txt line 532: > >> 530: >> 531: java/awt/Window/GetScreenLocation/GetScreenLocationTest.java 8225787 >> linux-x64 >> 532: java/awt/Dialog/MakeWindowAlwaysOnTop/MakeWindowAlwaysOnTop.java >> 8252813 macosx-aarch64 > > Looks like this bug is a closed one, can it be open? Maybe we should use JDK-8266243 instead. Other one has other tests as well. - PR: https://git.openjdk.java.net/jdk/pull/5044
Re: RFR: 5015261: JInternalFrame causes NPE [v2]
> JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Remove test - Update javadoc clarification - Changes: - all: https://git.openjdk.java.net/jdk/pull/4989/files - new: https://git.openjdk.java.net/jdk/pull/4989/files/2a2ab50d..0c96a6e3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=00-01 Stats: 73 lines in 2 files changed: 1 ins; 63 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v3]
On Thu, 5 Aug 2021 08:37:07 GMT, Prasanta Sadhukhan
wrote:
>> The Nimbus look and feel ignores the configured background color of a
>> JTextPane and always uses white.
>> Every other look and feel tested (Metal, Motif, and Windows) correctly
>> honors the configured background color of a JTextPane.
>> Issue seems to be in the hardcoded background color in nimbus skin which is
>> rectified via the fix.
>> After this fix, the background color is correctly updated to whatever is
>> being set via JTextPane.setBackground().
>>
>> CI all test run is green.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Use background componentPropertyName
Restored "shape" tag and now tackling the bug from the fact that
JTextField/JTextARea setbackground works unlike JTextPane/EditorPane.
It seems TextPanePainter class is generated from skin template where for
JTextField/Area we have
private void paintBackgroundEnabled(Graphics2D g) {
this.rect = this.decodeRect1();
g.setPaint((Color)this.componentColors[0]);
g.fill(this.rect);
}
but for JTextPane/EditorPane, we have
private void paintBackgroundEnabled(Graphics2D g) {
this.rect = this.decodeRect1();
g.setPaint((Color)this.color2);
g.fill(this.rect);
}
where color2 is NimubsLightBackground so it seems to always paint white.
Proposed fix is to add background componentPropertyName for ENABLED state (for
the fact
getContext is always called with SynthContext context = getContext(comp,
ENABLED);
so that correct background component color is painted.`
-
PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v3]
> The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Use background componentPropertyName - Changes: - all: https://git.openjdk.java.net/jdk/pull/4930/files - new: https://git.openjdk.java.net/jdk/pull/4930/files/ccde17d7..f308e231 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=01-02 Stats: 18 lines in 1 file changed: 14 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Wed, 4 Aug 2021 23:06:35 GMT, Sergey Bylokhov wrote: >> It looks the fix doesn't work or alternatively it works in an unexpected way. >> >> Both JEditorPane and JTextPane used to have white background but now, with >> these changes, their background is the same of the panel below. It's >> probably know what we want. > > The last time this code was touched we get a regression > https://bugs.openjdk.java.net/browse/JDK-8266510 which was not found by any > tests. JCK cannot found them since this is L&F dependent. And regression > tests do not have related tests because we had never bugs here, it worked > this way from the beginning. > > So it will be good to understand why these tags prevent the color change, how > the shape tags are used, instead of some unknown changes here and there. JDK-8266510 regression was found by SwingSet2 so it is wrong to say it was not found by any test, maybe it was not verified during fix of JDK-8249674. For this, all existing test is run. - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 5015261: JInternalFrame causes NPE
On Wed, 4 Aug 2021 09:02:25 GMT, Prasanta Sadhukhan wrote: > JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called > with null value. There are many places in JInternalFrame where > getDesktopIcon() is accessed without null check which might cause NPE. > Added null check for those cases. Will just stating null icon is unsupported do? I think in that case should we throw IAE? - PR: https://git.openjdk.java.net/jdk/pull/4989
Integrated: 4819544: SwingSet2 JTable Demo throws NullPointerException
On Tue, 3 Aug 2021 08:23:19 GMT, Prasanta Sadhukhan wrote: > In the JTable demo, if we double click on the first cell in the Favorite > Number column, delete the value and click on some > other cell, a java.lang.NullPointerException is getting thrown. > The flaw is in the TableDemo's TableModel getColumnClass() method which > queries the value of cell 0 in the target column, and returns its Class. This > logic seems to be flawed - an NPE will be thrown when the value in cell 0 is > null. > Fix by checking for null. This pull request has now been integrated. Changeset: 64d18d45 Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/64d18d45ef885d5cdd893b7f56236d3f22b4b84c Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod 4819544: SwingSet2 JTable Demo throws NullPointerException Reviewed-by: pbansal, aivanov - PR: https://git.openjdk.java.net/jdk/pull/4969
Re: RFR: 4819544: SwingSet2 JTable Demo throws NullPointerException [v2]
> In the JTable demo, if we double click on the first cell in the Favorite > Number column, delete the value and click on some > other cell, a java.lang.NullPointerException is getting thrown. > The flaw is in the TableDemo's TableModel getColumnClass() method which > queries the value of cell 0 in the target column, and returns its Class. This > logic seems to be flawed - an NPE will be thrown when the value in cell 0 is > null. > Fix by checking for null. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Use ternary operator - Changes: - all: https://git.openjdk.java.net/jdk/pull/4969/files - new: https://git.openjdk.java.net/jdk/pull/4969/files/5b181bee..fffb3f32 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4969&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4969&range=00-01 Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4969.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4969/head:pull/4969 PR: https://git.openjdk.java.net/jdk/pull/4969
Integrated: 6350025: API documentation for JOptionPane using deprecated methods.
On Mon, 2 Aug 2021 09:17:05 GMT, Prasanta Sadhukhan wrote: > The method show() in the JOptionPane javadoc example is deprecated. The > recommended method to call would be setVisible(true). This pull request has now been integrated. Changeset: 4df1bc4b Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/4df1bc4bc6ec19f9a5bb0e85ee34fb746011e1fa Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 6350025: API documentation for JOptionPane using deprecated methods. Reviewed-by: jdv - PR: https://git.openjdk.java.net/jdk/pull/4953
RFR: 5015261: JInternalFrame causes NPE
JInternalFrame.getDesktopIcon() can be null as setDesktopIcon() can be called with null value. There are many places in JInternalFrame where getDesktopIcon() is accessed without null check which might cause NPE. Added null check for those cases. - Commit messages: - 5015261: JInternalFrame causes NPE Changes: https://git.openjdk.java.net/jdk/pull/4989/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4989&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-5015261 Stats: 72 lines in 2 files changed: 63 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/4989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4989/head:pull/4989 PR: https://git.openjdk.java.net/jdk/pull/4989
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Tue, 3 Aug 2021 06:34:28 GMT, Prasanta Sadhukhan wrote: >> Does it mean that "shapes" tag is not used and we can remove all its usage >> in the file? > > I dont know. I am only telling by looking at source code..I dont know what > NimbusL&F creator meant it for... I think we should give this fix a chance...sInce this is solving 2 long-standing issue where setBackground() contract is not honoured plus causing no regression to existing tests. If we do find any testcase with "shape" in future from open community or any other related testcase being failed for this fix, we can always work on it (or at the least roll back this) - PR: https://git.openjdk.java.net/jdk/pull/4930
RFR: 4819544: SwingSet2 JTable Demo throws NullPointerException
In the JTable demo, if we double click on the first cell in the Favorite Number column, delete the value and click on some other cell, a java.lang.NullPointerException is getting thrown. The flaw is in the TableDemo's TableModel getColumnClass() method which queries the value of cell 0 in the target column, and returns its Class. This logic seems to be flawed - an NPE will be thrown when the value in cell 0 is null. Fix by checking for null. - Commit messages: - 4819544: SwingSet2 JTable Demo throws NullPointerException - 4819544: SwingSet2 JTable Demo throws NullPointerException Changes: https://git.openjdk.java.net/jdk/pull/4969/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4969&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-4819544 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4969.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4969/head:pull/4969 PR: https://git.openjdk.java.net/jdk/pull/4969
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Tue, 3 Aug 2021 06:28:59 GMT, Sergey Bylokhov wrote: >> Yes, I meant the grey background what is being described in >> EditorPane.background and TextPane.background is what is being shown now, >> not literally NimbusBlueGrey. Till now we were showing white which is in >> contrary to this default. >> >> I could not find any purpose of shape by looking at skin.laf as it is not >> being used in Synth source code, so that is why I ran all JCK/jtreg >> testsuite and found no regression. It is also there from initial days. >> I leave it for user of Synth/Nimbus L&F or anyone from open community to >> highlight the need of "shape" > > Does it mean that "shapes" tag is not used and we can remove all its usage in > the file? I dont know. I am only telling by looking at source code..I dont know what NimbusL&F creator meant it for... - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Tue, 3 Aug 2021 06:03:21 GMT, Sergey Bylokhov wrote: >> I have looked at >> https://docs.oracle.com/javase/tutorial/uiswing/lookandfeel/_nimbusDefaults.html >> where default color for >> EditorPane.background and TextPane.background is NimbusBlueGrey and not >> White what we have now, so I think we are having what is being specified in >> nimbus-default. >> SwingSet2 background color for the demos are same before and after the fix. >> I am not sure about the difference between background and shape but I have >> ran full JCK/JTREG tests and all are fine. I also tried passing Nimbus L&F >> via CI script for JCK/jtreg tests and those are also same before and after >> fix. > > In the linke above the > nimbusBlueGrey#a9b0be (169,176,190) > While the "EditorPane.background and TextPane.background" are: > TextPane.background #d6d9df (214,217,223) > EditorPane.background #d6d9df (214,217,223) > > It is still unclear why setting these values in the XML file prevents the > user to change it later, also it will be good to understand the purpose of > "shape" since it also sets some coordinates values. Yes, I meant the grey background what is being described in EditorPane.background and TextPane.background is what is being shown now, not literally NimbusBlueGrey. Till now we were showing white which is in contrary to this default. I could not find any purpose of shape by looking at skin.laf as it is not being used in Synth source code, so that is why I ran all JCK/jtreg testsuite and found no regression. It is also there from initial days. I leave it for user of Synth/Nimbus L&F or anyone from open community to highlight the need of "shape" - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Mon, 2 Aug 2021 22:13:24 GMT, Sergey Bylokhov wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Fix >> - JEditorpane background color fix > > src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf line 25750: > >> (failed to retrieve contents of file, check the PR for context) > What is the difference between background and shape tags? I have looked at https://docs.oracle.com/javase/tutorial/uiswing/lookandfeel/_nimbusDefaults.html where default color for EditorPane.background and TextPane.background is NimbusBlueGrey and not White what we have now, so I think we are having what is being specified in nimbus-default. SwingSet2 background color for the demos are same before and after the fix. I am not sure about the difference between background and shape but I have ran full JCK/JTREG tests and all are fine. I also tried passing Nimbus L&F via CI script for JCK/jtreg tests and those are also same before and after fix. - PR: https://git.openjdk.java.net/jdk/pull/4930
RFR: 6350025: API documentation for JOptionPane using deprecated methods.
The method show() in the JOptionPane javadoc example is deprecated. The recommended method to call would be setVisible(true). - Commit messages: - 6350025: API documentation for JOptionPane using deprecated methods. Changes: https://git.openjdk.java.net/jdk/pull/4953/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4953&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6350025 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4953/head:pull/4953 PR: https://git.openjdk.java.net/jdk/pull/4953
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
On Fri, 30 Jul 2021 20:42:57 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix
>> - JEditorpane background color fix
>
> test/jdk/javax/swing/plaf/nimbus/TestNimbusJTextPaneColor.java line 74:
>
>> 72: if (frame != null) {
>> 73: SwingUtilities.invokeAndWait(frame::dispose);
>> 74: }
>
> `frame` is accessed on main thread without synchronisation.
NimbusLookAndFeel is customized look and feel but I don't see mention of
default background color to be white in spec..If it is not set explicitly,
background color is drawn in "NimbusBlueGrey" background which seems to be same
as what is used in SwingSet2 demos in Nimbus L&F
-
PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color [v2]
> The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - Fix - JEditorpane background color fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/4930/files - new: https://git.openjdk.java.net/jdk/pull/4930/files/920f71dc..ccde17d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=00-01 Stats: 116 lines in 3 files changed: 97 ins; 8 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8058704: Nimbus does not honor JTextPane background color
On Fri, 30 Jul 2021 20:39:24 GMT, Alexey Ivanov wrote: > > > Does the JEditorPane have the same issue? > https://bugs.openjdk.java.net/browse/JDK-6789980 Yes, fixed - PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8049301: Suspicious use of string identity checks in JComponent.setUIProperty [v2]
On Fri, 30 Jul 2021 15:17:41 GMT, Alexander Zvegintsev wrote: >> I have taken care of NPE..I am not sure of switch which probably might be a >> hindrance in backporting if needed to earlier release trains. >> CI is also run..link in JBS. > >> I am not sure of switch which probably might be a hindrance in backporting >> if needed to earlier release trains. > > You still can use the old switch statement with breaks. It will not be consistent if we use switch only in this place because in most places, we either use the way we have before fix or or what I have done in version 1 or what I have done now. Probably, we need a PR to change other places where we have use pre-fix ways. - PR: https://git.openjdk.java.net/jdk/pull/4943
Integrated: 8049301: Suspicious use of string identity checks in JComponent.setUIProperty
On Fri, 30 Jul 2021 06:04:17 GMT, Prasanta Sadhukhan wrote: > JComponent.setUIProperty method uses string identity check (==) rather than > string equality checks (.equals) when comparing against the property name. > This is suspicious since string identity and equality and equivalent only for > interned strings. > Rectified to use String.equals() check. This pull request has now been integrated. Changeset: baf7797b Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/baf7797b092704d7dc9401db8b3dda19b91be303 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8049301: Suspicious use of string identity checks in JComponent.setUIProperty Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/4943
Re: RFR: 8049301: Suspicious use of string identity checks in JComponent.setUIProperty [v2]
On Fri, 30 Jul 2021 09:32:10 GMT, Alexander Zvegintsev
wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Prevent NPE
>
> src/java.desktop/share/classes/javax/swing/JComponent.java line 4194:
>
>> 4192: */
>> 4193: void setUIProperty(String propertyName, Object value) {
>> 4194: if (propertyName.equals("opaque")) {
>
> This will throw an NPE if `propertyName` is `null`, which didn't happened
> before the fix.
>
>> this method could be rewritten to use strings in switch.
>
> Probably we might still want to use switch, it will handle the NPE case as
> before the fix.
>
> Did you run CI tests after the fix?
I have taken care of NPE..I am not sure of switch which probably might be a
hindrance in backporting if needed to earlier release trains.
CI is also run..link in JBS.
-
PR: https://git.openjdk.java.net/jdk/pull/4943
Re: RFR: 8049301: Suspicious use of string identity checks in JComponent.setUIProperty [v2]
> JComponent.setUIProperty method uses string identity check (==) rather than > string equality checks (.equals) when comparing against the property name. > This is suspicious since string identity and equality and equivalent only for > interned strings. > Rectified to use String.equals() check. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Prevent NPE - Changes: - all: https://git.openjdk.java.net/jdk/pull/4943/files - new: https://git.openjdk.java.net/jdk/pull/4943/files/bd43899a..62418e5b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4943&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4943&range=00-01 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4943.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4943/head:pull/4943 PR: https://git.openjdk.java.net/jdk/pull/4943
Re: RFR: 8270058: Use Objects.check{Index, FromToIndex, FromIndexSize} for java.desktop [v2]
On Tue, 27 Jul 2021 02:46:53 GMT, Yi Yang wrote: >> I meant >> >> if (imageIndex < minIndex) >> throw new IndexOutOfBoundsException(); >> >> can't it be replaced with Objects.checkFromToIndex(minIndex, imageIndex, .. > > Hi, sorry for the late response. checkFromToIndex has many constraints, this > code may not satisfy those constraints, so I don't think we should do that in > this PR.(Maybe we can file a new PR to do replacement carefully) OK...The subject line of this PR states that checkFromToIndex will also be used but I see only checkIndex so I intervened. Anyways, the code in question is in test so not much of an issue. - PR: https://git.openjdk.java.net/jdk/pull/4718
Re: RFR: 8270058: Use Objects.check{Index, FromToIndex, FromIndexSize} for java.desktop [v3]
On Thu, 22 Jul 2021 06:29:13 GMT, Yi Yang wrote: >> After JDK-8265518(#3615), it's possible to replace all variants of >> checkIndex by >> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in >> the whole JDK codebase. >> >> As Mandy suggested, I create this PR for changes involving client code: >> >> src/java.desktop/share/classes/com/sun/media/sound/AudioFloatInputStream.java >> src/java.desktop/share/classes/javax/swing/JTabbedPane.java >> test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java >> test/jdk/javax/imageio/AppletResourceTest.java >> test/jdk/javax/imageio/ImageReaderReadAll.java >> test/jdk/javax/imageio/metadata/IIOMetadataFormat/UserPluginMetadataFormatTest.java > > Yi Yang has updated the pull request incrementally with one additional commit > since the last revision: > > typo Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4718
RFR: 8049301: Suspicious use of string identity checks in JComponent.setUIProperty
JComponent.setUIProperty method uses string identity check (==) rather than string equality checks (.equals) when comparing against the property name. This is suspicious since string identity and equality and equivalent only for interned strings. Rectified to use String.equals() check. - Commit messages: - 8049301: Suspicious use of string identity checks in JComponent.setUIProperty Changes: https://git.openjdk.java.net/jdk/pull/4943/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4943&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8049301 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4943.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4943/head:pull/4943 PR: https://git.openjdk.java.net/jdk/pull/4943
Re: RFR: 8058704: Nimbus does not honor JTextPane background color
On Thu, 29 Jul 2021 11:44:31 GMT, Prasanta Sadhukhan wrote: > The Nimbus look and feel ignores the configured background color of a > JTextPane and always uses white. > Every other look and feel tested (Metal, Motif, and Windows) correctly honors > the configured background color of a JTextPane. > Issue seems to be in the hardcoded background color in nimbus skin which is > rectified via the fix. > After this fix, the background color is correctly updated to whatever is > being set via JTextPane.setBackground(). > > CI all test run is green. test/jdk/javax/swing/plaf/nimbus/TestNimbusJTextPaneColor.java line 53: > 51: JTextPane tp = new JTextPane(); > 52: tp.setForeground(Color.WHITE); > 53: tp.setBackground(Color.BLACK); There's a brief flash of white background before the actual and proper background color is set. Since the functionality of JTextPane background is rectified/restored, I have raised the PR and I will work on the initial white flashing in a subsequent PR. - PR: https://git.openjdk.java.net/jdk/pull/4930
RFR: 8058704: Nimbus does not honor JTextPane background color
The Nimbus look and feel ignores the configured background color of a JTextPane and always uses white. Every other look and feel tested (Metal, Motif, and Windows) correctly honors the configured background color of a JTextPane. Issue seems to be in the hardcoded background color in nimbus skin which is rectified via the fix. After this fix, the background color is correctly updated to whatever is being set via JTextPane.setBackground(). CI all test run is green. - Commit messages: - Fix - Fix - Fix - 8058704: Nimbus does not honor JTextPane background color Changes: https://git.openjdk.java.net/jdk/pull/4930/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4930&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8058704 Stats: 84 lines in 2 files changed: 77 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4930.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4930/head:pull/4930 PR: https://git.openjdk.java.net/jdk/pull/4930
Re: RFR: 8271396: Spelling errors
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg wrote: > This PR fixes the following spelling errors: > > choosen -> chosen > commad -> command > hiearchy -> hierarchy > leagacy -> legacy > minium -> minimum > subsytem -> subsystem > unamed -> unnamed Thanks for awt/swing correction. - Marked as reviewed by psadhukhan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2385
Integrated: 8137101: [TEST_BUG] javax/swing/plaf/basic/BasicHTML/4251579/bug4251579.java failure due to timing
On Wed, 28 Jul 2021 07:24:38 GMT, Prasanta Sadhukhan wrote: > This test was failing in CI test run due to timing issue. Adjusted robot > delay..Also, made frame to show in middle of screen and dispose of frame at > end. > CI test run is green. This pull request has now been integrated. Changeset: dcdb1b6a Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/dcdb1b6aaad912669614ff52ad2b0b8804f204d1 Stats: 51 lines in 2 files changed: 18 ins; 5 del; 28 mod 8137101: [TEST_BUG] javax/swing/plaf/basic/BasicHTML/4251579/bug4251579.java failure due to timing Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/4918
RFR: 8137101: [TEST_BUG] javax/swing/plaf/basic/BasicHTML/4251579/bug4251579.java failure due to timing
This test was failing in CI test run due to timing issue. Adjusted robot delay..Also, made frame to show in middle of screen and dispose of frame at end. CI test run is green. - Commit messages: - Fix - 8137101: [TEST_BUG] javax/swing/plaf/basic/BasicHTML/4251579/bug4251579.java failure due to timing Changes: https://git.openjdk.java.net/jdk/pull/4918/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4918&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8137101 Stats: 51 lines in 2 files changed: 18 ins; 5 del; 28 mod Patch: https://git.openjdk.java.net/jdk/pull/4918.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4918/head:pull/4918 PR: https://git.openjdk.java.net/jdk/pull/4918
Re: RFR: 8266510: Nimbus JTree default tree cell renderer does not use selected text color
On Mon, 26 Jul 2021 05:46:44 GMT, Tejpal Rebari wrote: > Please review the following fix for JDK-18. > This is revert of the fix for nimbus jtree renderer properties persist > across L&F(https://bugs.openjdk.java.net/browse/JDK-8249674), This fixed > caused nimbus jtree default tree cell renderer to not use the selected text > color, which was white originally and changed to black after the fix. > Reverted the change to match original behaviour. > > Tested on MacOS , Windows and Linux. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4903
Re: RFR: 8270058: Use Objects.check{Index, FromToIndex, FromIndexSize} for java.desktop [v2]
On Thu, 22 Jul 2021 06:23:43 GMT, Yi Yang wrote:
>> test/jdk/javax/imageio/ImageReaderReadAll.java line 94:
>>
>>> 92: if (seekForwardOnly) {
>>> 93: if (imageIndex < minIndex)
>>> 94: throw new IndexOutOfBoundsException();
>>
>> Can't we use checkFromToIndex for this IOOBE too? Used in many other places
>> in this PR...
>>
>> Please also confirm all tests modified are green after this modification..
>
> checkFromToIndex has different semantice with checkIndex:
>
> - checkFromToIndex
>
> *
> * {@code fromIndex < 0}
> * {@code fromIndex > toIndex}
> * {@code toIndex > length}
> * {@code length < 0}, which is implied from the former
> inequalities
> *
>
> - checkIndex
>
> *
> * {@code index < 0}
> * {@code index >= length}
> * {@code length < 0}, which is implied from the former
> inequalities
> *
>
> ---
>
> The following tests are passed after applying this patch:
>
> test/jdk/javax/imageio/AppletResourceTest.java
> test/jdk/javax/imageio/ImageReaderReadAll.java
> test/jdk/javax/imageio/metadata/IIOMetadataFormat/UserPluginMetadataFormatTest.java
>
>
> ClippedImages.java is a manual test, I have no environment to test it, but
> since it's a trivial replacement, I think it's okay..
I meant
if (imageIndex < minIndex)
throw new IndexOutOfBoundsException();
can't it be replaced with Objects.checkFromToIndex(minIndex, imageIndex, ..
-
PR: https://git.openjdk.java.net/jdk/pull/4718
Re: RFR: 8270058: Use Objects.check{Index, FromToIndex, FromIndexSize} for java.desktop [v2]
On Wed, 21 Jul 2021 09:58:23 GMT, Yi Yang wrote:
>> After JDK-8265518(#3615), it's possible to replace all variants of
>> checkIndex by
>> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in
>> the whole JDK codebase.
>>
>> As Mandy suggested, I create this PR for changes involving client code:
>>
>> src/java.desktop/share/classes/com/sun/media/sound/AudioFloatInputStream.java
>> src/java.desktop/share/classes/javax/swing/JTabbedPane.java
>> test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java
>> test/jdk/javax/imageio/AppletResourceTest.java
>> test/jdk/javax/imageio/ImageReaderReadAll.java
>> test/jdk/javax/imageio/metadata/IIOMetadataFormat/UserPluginMetadataFormatTest.java
>
> Yi Yang has updated the pull request incrementally with one additional commit
> since the last revision:
>
> checkFromIndexSize(off, len,...)
test/jdk/javax/imageio/ImageReaderReadAll.java line 94:
> 92: if (seekForwardOnly) {
> 93: if (imageIndex < minIndex)
> 94: throw new IndexOutOfBoundsException();
Can't we use checkFromToIndex for this IOOBE too? Used in many other places in
this PR...
Please also confirm all tests modified are green after this modification..
-
PR: https://git.openjdk.java.net/jdk/pull/4718
Re: RFR: 8268284: javax/swing/JComponent/7154030/bug7154030.java fails with "Exception: Failed to hide opaque button"
On Thu, 15 Jul 2021 11:27:39 GMT, Alexander Zuev wrote: > Reducing the height of the analyzed area to get rid of the rounded edges > at the botom of the frame. Apparently shadow outside of it being > rendered by Mac OS X Big Sur inconsistently which gives random test > failures on M1 macs with Big Sur. test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 110: > 108: locy = bounds.y + insets.top; > 109: frw = bounds.width - insets.left - insets.right; > 110: frh = bounds.height - insets.top - insets.bottom - 25; Will making frame undecorated help instead of this? Can you also please give a CI job running this test on all platforms for several iterations? Since this test has undergone multiple iterations, can you please confirm that this test still fails without 7154030 fix? - PR: https://git.openjdk.java.net/jdk/pull/4792
Re: [jdk17] RFR: 8269529: javax/swing/reliability/HangDuringStaticInitialization.java fails in Windows debug build [v3]
On Tue, 29 Jun 2021 08:29:29 GMT, Christoph Langer wrote: >> Exclude test on Windows if VM is from a debug build. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Update Copyright year Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/160
Re: [jdk17] RFR: 8269529: javax/swing/reliability/HangDuringStaticInitialization.java fails in Windows debug build [v2]
On Mon, 28 Jun 2021 15:43:35 GMT, Christoph Langer wrote: >> Exclude test on Windows if VM is from a debug build. > > Christoph Langer has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. looks ok. It will be good to update the copyright date too... - PR: https://git.openjdk.java.net/jdk17/pull/160
Integrated: 8264218: Public method javax.swing.JMenu.setComponentOrientation() has no spec
On Fri, 26 Mar 2021 11:30:35 GMT, Prasanta Sadhukhan wrote: > A public overriding method > JMenu.setComponentOrientation(java.awt.ComponentOrientation) > has no spec. > Added spec for the method. This pull request has now been integrated. Changeset: e48d7d66 Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/e48d7d66582d9c9630d85e86ff344794656914fc Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod 8264218: Public method javax.swing.JMenu.setComponentOrientation() has no spec Reviewed-by: aivanov - PR: https://git.openjdk.java.net/jdk/pull/3213
Integrated: 8260331: javax/swing/JInternalFrame/8146321/JInternalFrameIconTest.java failed with "ERROR: icon and imageIcon not same."
On Mon, 17 May 2021 09:05:06 GMT, Prasanta Sadhukhan wrote: > This testcase has failed intermittently in CI testing citing icon and > ImageIcon images are not same. > It's been observed that the rectangle been passed to > Robot.createScreenCapture was incorrect to compare the icon images but is > passing because the images were not different (but the images itself was > incorrect) so not reliable. > > When it fails, it is seen that the icon and imageicon images captured, with > height of the screenCapture which is the value of the topinset of > internalFrame, is wrong and differ in some pixels at the end > `icon captureRect java.awt.Rectangle[x=715,y=366,width=500,height=5]` > The failed images look like (where one can see that there is some difference > at the end of line) > icon image >  > image icon >  > > which is not compring the icon images actually. > > Rectified screenrect to correctly compare the icons. After the fix, the > images being compared are these > icon image >  > imageicon >  > > Several iterations of the modified test execution is green on all platforms. > Link in JBS. > I have also verified the modified test fails without 8146321 fix. This pull request has now been integrated. Changeset: 39a454bb Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/39a454bb879fe316a69a4ec33ab287db2b5837db Stats: 11 lines in 2 files changed: 4 ins; 1 del; 6 mod 8260331: javax/swing/JInternalFrame/8146321/JInternalFrameIconTest.java failed with "ERROR: icon and imageIcon not same." Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/4048
