Integrated: 8333940: Ensure javax/swing/TestUngrab.java run on all platforms
On Tue, 11 Jun 2024 06:05:02 GMT, Prasanta Sadhukhan wrote: > TestUngrab.java was added for > [JDK-8267374](https://bugs.openjdk.org/browse/JDK-8267374) macos fix but the > test can be run for all platforms as the behaviour is common for all > CI run job link in JBS.. This pull request has now been integrated. Changeset: 1c80ddb8 Author: Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/1c80ddb8efdb883623652b20849413b602c10c36 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod 8333940: Ensure javax/swing/TestUngrab.java run on all platforms Reviewed-by: tr - PR: https://git.openjdk.org/jdk/pull/19644
Re: RFR: 8333940: Ensure javax/swing/TestUngrab.java run on all platforms
On Wed, 12 Jun 2024 04:12:13 GMT, Tejesh R wrote: >> TestUngrab.java was added for >> [JDK-8267374](https://bugs.openjdk.org/browse/JDK-8267374) macos fix but the >> test can be run for all platforms as the behaviour is common for all >> CI run job link in JBS.. > > test/jdk/javax/swing/JMenu/TestUngrab.java line 28: > >> 26: /* >> 27: * @test >> 28: * @bug 8267374 > > Should the current bug id be updated here? No, we only put the product ids in the tag and this is test-only fix - PR Review Comment: https://git.openjdk.org/jdk/pull/19644#discussion_r1635790022
RFR: 8333940: Ensure javax/swing/TestUngrab.java run on all platforms
TestUngrab.java was added for [JDK-8267374](https://bugs.openjdk.org/browse/JDK-8267374) macos fix but the test can be run for all platforms as the behaviour is common for all CI run job link in JBS.. - Commit messages: - Test-fix Changes: https://git.openjdk.org/jdk/pull/19644/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19644=00 Issue: https://bugs.openjdk.org/browse/JDK-8333940 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19644.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19644/head:pull/19644 PR: https://git.openjdk.org/jdk/pull/19644
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v7]
On Mon, 10 Jun 2024 20:01:47 GMT, Alisen Chung wrote: >> Issue is a mouse drag will trigger a popup in macos, but not in linux or >> windows. >> The solution is to add a check for a mouse pressed event to show popup and >> prevent mouse entered events from triggering the popup > > Alisen Chung has updated the pull request incrementally with one additional > commit since the last revision: > > updated test, removed robot waitForIdles, jtreg summary, release in finally Marked as reviewed by psadhukhan (Reviewer). test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 39: > 37: * @test > 38: * @bug 8315655 > 39: * @summary Right click and dragging over a component with a popup menu > will open the popup Actually summary should be what test is expected to do like "Verifies Right click and dragging over a component with a popup menu will not open the popup" test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 49: > 47: static Robot robot; > 48: static volatile Point srcPoint; > 49: static volatile Dimension d; better to club all volatile vars one after another.. - PR Review: https://git.openjdk.org/jdk/pull/19569#pullrequestreview-2109410215 PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1634246763 PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1634248178
Re: RFR: 8233177: Remove testcase for JDK-8001470 as fix has been reverted [v3]
On Tue, 13 Feb 2024 07:50:15 GMT, Prasanta Sadhukhan wrote: >> Existing regression test is failing because textfield height is not as per >> test's expectation..Seems like the indic character being tried to render is >> not being loaded (probably because of missing glyphs) leading to 0 >> preferredSpan from >> [BoxView](https://github.com/openjdk/jdk/blame/5a74c2a67ebcb47e51732f03c4be694bdf920469/src/java.desktop/share/classes/javax/swing/text/BoxView.java#L545-L552) >> which is superclass for `i18nFieldVIew`, the field view for Indic >> characters. >> Tried block character in the test which is now being loaded properly leading >> to correct height.. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Test removed as corresponding fix is reverted keep alive - PR Comment: https://git.openjdk.org/jdk/pull/17528#issuecomment-2157333247
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v6]
On Fri, 7 Jun 2024 21:19:41 GMT, Alisen Chung wrote:
>> Issue is a mouse drag will trigger a popup in macos, but not in linux or
>> windows.
>> The solution is to add a check for a mouse pressed event to show popup and
>> prevent mouse entered events from triggering the popup
>
> Alisen Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update test based on feedback
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 40:
> 38: * @bug 8315655
> 39: * @key headful
> 40: * @run main MouseDragPopupTest
summary tag missing
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 45:
> 43: static volatile boolean failed;
> 44: static volatile JFrame frame;
> 45: static volatile JPanel panel;
frame and panel are/should be accessed in EDT only so no need for volatile
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 51:
> 49: public static void main(String[] args) throws Exception {
> 50: try {
> 51: failed = false;
By default boolean class variable is false, no need to set explicitly..
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 71:
> 69: dstPoint.translate(4 * d.width / 15, 0);
> 70:
> 71:
unnecessary line
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 84:
> 82: robot.waitForIdle();
> 83:
> 84: robot.mouseRelease(InputEvent.BUTTON3_DOWN_MASK);
Better to release in `finally` block so that if unexpectedly any exception
occurs it will not hamper any other test if tun as part of CI job..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1632616431
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1632616227
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1632615767
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1632616684
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1632618131
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v4]
On Fri, 7 Jun 2024 07:20:35 GMT, Prasanta Sadhukhan wrote: >> I believe only MouseEvents can be popup triggers. I checked in MouseEvent >> that there is a field for popup triggers and no such field in KeyEvent. >> >> The isPopupTrigger method here also previously checked button down masks >> already, so adding a check for the correct mouse events shouldn't be an >> issue even if there was a way to show a popup menu with a different event >> type. > > looks good.. Although there's one more issue it seems where in native, after popup is shown when right mouse button (or CTRL+left mouse button) is pressed, if you release the mouse the popup vanishes which I noticed in Notes, Mail but it does not happen in JDK and popup remains open...but it seems not related to this...did you notice? - PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630781731
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v5]
On Thu, 6 Jun 2024 18:28:12 GMT, Alisen Chung wrote: >> Issue is a mouse drag will trigger a popup in macos, but not in linux or >> windows. >> The solution is to add a check for a mouse pressed event to show popup and >> prevent mouse entered events from triggering the popup > > Alisen Chung has updated the pull request incrementally with three additional > commits since the last revision: > > - update copyright years > - automated test > - changed test to automatic test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 39: > 37: * @test > 38: * @bug 8315655 > 39: * @requires (os.family == "mac") Also, is there any need to restrict it to mac as I guess in other platforms, this test should anyway pass...Did you check? - PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630776745
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v4]
On Thu, 6 Jun 2024 18:41:16 GMT, Alisen Chung wrote:
>> src/java.desktop/macosx/classes/sun/lwawt/macosx/NSEvent.java line 276:
>>
>>> 274: && jeventType != MouseEvent.MOUSE_RELEASED) {
>>> 275: return false;
>>> 276: }
>>
>> Just to double check, are the only jeventTypes to trigger popups using
>> mouse? Could there be other event types, maybe using keyboard for example
>> with tab/arrows & enter?
>
> I believe only MouseEvents can be popup triggers. I checked in MouseEvent
> that there is a field for popup triggers and no such field in KeyEvent.
>
> The isPopupTrigger method here also previously checked button down masks
> already, so adding a check for the correct mouse events shouldn't be an issue
> even if there was a way to show a popup menu with a different event type.
looks good..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630760813
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v5]
On Thu, 6 Jun 2024 18:28:12 GMT, Alisen Chung wrote:
>> Issue is a mouse drag will trigger a popup in macos, but not in linux or
>> windows.
>> The solution is to add a check for a mouse pressed event to show popup and
>> prevent mouse entered events from triggering the popup
>
> Alisen Chung has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - update copyright years
> - automated test
> - changed test to automatic
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 44:
> 42: */
> 43: public class MouseDragPopupTest {
> 44: static boolean failed;
volatile
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 48:
> 46: static JPanel panel;
> 47: static JPanel innerPanel;
> 48: static JPopupMenu menu;
guess these 2 can be local vars..
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 54:
> 52: public static void main(String[] args) throws Exception {
> 53: SwingUtilities.invokeAndWait(() -> {
> 54: createAndShowGUI();
normally after frame is created, we do robot.waitForIdle and robot.delay(1000)
before accessing the frame location or test other things..
test/jdk/javax/swing/JPopupMenu/MouseDragPopupTest.java line 83:
> 81:
> 82: if (failed) {
> 83: throw new RuntimeException("Popup was shown, Test Failed.");
frame needs to be disposed at end..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630760974
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630762974
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630761930
PR Review Comment: https://git.openjdk.org/jdk/pull/19569#discussion_r1630763411
Re: RFR: 8315655: [macos] Right click and dragging over a component with a popup menu will open the popup [v2]
On Thu, 6 Jun 2024 05:26:53 GMT, Alisen Chung wrote: >> Issue is a mouse drag will trigger a popup in macos, but not in linux or >> windows. >> The solution is to add a check for a mouse pressed event to show popup and >> prevent mouse entered events from triggering the popup > > Alisen Chung has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains six additional > commits since the last revision: > > - Merge branch 'master' of github.com:openjdk/jdk into 8315655 > - fix trailing whitespace in instructions string > - fix line ender > - fix whitespaces and newline > - fix whitespaces and newline > - init commit I guess it needs to be understood why it is not failing in windows and linux and only reproducible in macos. The fix in BasicLookAndFeel is not appropriate in my opinion as that shared code will be exercised in windows and linux too.. I guess we need to find out why `BasicLookAndFeel.evenDIspatched` is called for MOUSE_PRESSED in macos and not in windows/linux (ie only called for MOUSE_RELEASED event), maybe something needs to be done in macos JDK native event processing to ensure it is only called for "mouse release" event for showing popup. Also, the test can be automated as it is about popup being shown or not which can be easily checked by `isPopupMenuVIsible/isVisible` method - PR Comment: https://git.openjdk.org/jdk/pull/19569#issuecomment-2151571243
Re: RFR: 8332866: Crash in ImageIO JPEG decoding when MEM_STATS in enabled
On Fri, 24 May 2024 08:37:25 GMT, Jayathirth D V wrote: > In IJG library's jmemmgr.c file we can define MEM_STATS(by default this flag > is disabled and we don't see this issue) to enable printing of memory trace > logs when we have OOM. But if we enable it we get crash while disposing IJG > stored objects in jmemmgr->free-pool() function. > > This is happening because we delete the error handler before we actually > start deleting IJG stored objects and while freeing the IJG objects we try to > access cinfo->err->trace_level of error handler. This early deletion of error > handler is happening in imageioJPEG.c->imageio_dispose() function. > > Moved the logic to delete error handler after we are done with deleting IJG > stored objects, after this change there is no crash. There is no regression > test because this issue is seen only when we enable MEM_STATS flag in IJG > library. Ran jtreg ImageIO tests with code update and i don't see any > regressions. > > I have verified that this issue doesn't effect SplashScreen code path and > disposing of IJG objects is handled differently in SplashScreen. Please update the copyright year when you integrate - PR Comment: https://git.openjdk.org/jdk/pull/19386#issuecomment-2146515779
Integrated: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized
On Thu, 30 May 2024 12:05:25 GMT, Prasanta Sadhukhan wrote: > Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is > resized from the lower right corner, then the Menu / JPopupmenu stays open > unlike in native osx apps like Notes, Mail etc.. > > This is because when LMouseButton is pressed on non-client area, the window > should get a UngrabEvent for it to close/cancel the popupmenu (as is done in > [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) > but it was seen that the mac AWTWindow code only recognizes the title-bar as > the non-client area so > [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) > is not called. > Fix is made to recognize the edges of the window as non-client area This pull request has now been integrated. Changeset: 1c514b34 Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/1c514b34c0260823e70f209996ac933a76ac34c2 Stats: 125 lines in 2 files changed: 124 ins; 0 del; 1 mod 8325435: [macos] Menu or JPopupMenu not closed when main window is resized Reviewed-by: azvegint - PR: https://git.openjdk.org/jdk/pull/19474
Re: RFR: 8332866: Crash in ImageIO JPEG decoding when MEM_STATS in enabled
On Fri, 24 May 2024 08:37:25 GMT, Jayathirth D V wrote: > In IJG library's jmemmgr.c file we can define MEM_STATS(by default this flag > is disabled and we don't see this issue) to enable printing of memory trace > logs when we have OOM. But if we enable it we get crash while disposing IJG > stored objects in jmemmgr->free-pool() function. > > This is happening because we delete the error handler before we actually > start deleting IJG stored objects and while freeing the IJG objects we try to > access cinfo->err->trace_level of error handler. This early deletion of error > handler is happening in imageioJPEG.c->imageio_dispose() function. > > Moved the logic to delete error handler after we are done with deleting IJG > stored objects, after this change there is no crash. There is no regression > test because this issue is seen only when we enable MEM_STATS flag in IJG > library. Ran jtreg ImageIO tests with code update and i don't see any > regressions. > > I have verified that this issue doesn't effect SplashScreen code path and > disposing of IJG objects is handled differently in SplashScreen. Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19386#pullrequestreview-2092897041
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v17]
On Thu, 30 May 2024 18:45:38 GMT, Alexander Scherbatiy wrote: >> The fix adds new public `OutputBin` print attribute class which allow to set >> a printer output bin in a `PrinterJob` class. The corresponding internal >> `CustomOutputBin` class is added as well. >> >> - Constants used in `OutputBin` class are based on [Internet Printing >> Protocol (IPP): “output-bin” attribute >> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf) >> document. >> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD >> `ppdFindOption(..., "OutputBin")` function to get supported output bins for >> the given printer on native level. >> - The fix propagates the `OutputBin` attribute from the printer job >> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS. >> >> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where >> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice): >> - Printer settings, None >> - Inner tray, INNERTRAY >> - Separator tray, SEPARATORTRAY >> - Finisher (face-down), Main >> >> if `Printer settings`, `Inner tray`, or `Finisher (face-down)` >> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is >> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If >> `Separator tray` is used a page is printed to the Separator tray. This is >> consistent with the printer behavior when a native print dialog is used from >> a native Preview app to print a document on macOS. > > Alexander Scherbatiy has updated the pull request incrementally with one > additional commit since the last revision: > > Add Skip test option to the OutputBinAttributePrintDialogTest with the > native dialog Overall Looks good to me although I could not test it fully to my liking owing to non-support of output-bin in my printer.. - Marked as reviewed by psadhukhan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16166#pullrequestreview-2092883186
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Fri, 31 May 2024 18:57:03 GMT, Alexander Scherbatiy wrote: >> If it's only about adding tray-1 to tray-11 in the above list and not about >> whole lot of changes in other areas, so that supported list contains those >> "tray-N" values, I guess we should add them.. >> I guess isSupportedAttributedValues(tray-N) will not return false in that >> case.. > > Suppose `getSupportedAttributeValues(OutputBin.class, null, null)` returns > [`top`, `face-down`] output bins. > `isSupportedAttributedValues(...)` returns `true` for `OutputBin.TOP` and > `OutputBin.FACE_DOWN` constants and `false` for `OutputBin.TRAY_1` if it was > added in current implementation. > > To make a one to one correspondence between `OutputBin.TOP` and > `OutputBin.TRAY_1` constants it needs to add some logic and may be even make > them equal in terms of `OutputBin` class which extends `EnumSyntax` class. > It is not clear should such logic be provided in the current release or it is > better to provide its proper implementation in the next release if necessary. ok - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1623839809
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 14:49:45 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Non-client aread edge area modified, test format
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:
>
>> 1029: (p.x >= (frame.origin.x + contentRect.size.width - 3))
>> ||
>> 1030: (fabs(frame.origin.x - p.x) < 3) ||
>> 1031: (fabs(frame.origin.y - p.y) < 3)) {
>
> Does `fabs` use float? It makes code more compact but it may be imprecise and
> less performant than integer arithmetics.
Yes..https://man7.org/linux/man-pages/man3/fabs.3.html and [CGFLoat
](https://developer.apple.com/documentation/corefoundation/cgfloat?language=objc)
can be double/float and fabs can accomodate both, in this case it uses
double...We have tested on 3 different systems without any issue so would keep
it for now until we find any adverse effect..
> test/jdk/javax/swing/JMenu/TestUngrab.java line 101:
>
>> 99: robot.delay(1000);
>> 100: System.out.println("isPopupMenuVisible " +
>> menu.isPopupMenuVisible());
>> 101: if (menu.isPopupMenuVisible()) {
>
> `menu.isPopupMenuVisible()` should also be accessed on EDT.
Yes...done..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1623826880
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1623825295
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v3]
> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is > resized from the lower right corner, then the Menu / JPopupmenu stays open > unlike in native osx apps like Notes, Mail etc.. > > This is because when LMouseButton is pressed on non-client area, the window > should get a UngrabEvent for it to close/cancel the popupmenu (as is done in > [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) > but it was seen that the mac AWTWindow code only recognizes the title-bar as > the non-client area so > [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) > is not called. > Fix is made to recognize the edges of the window as non-client area Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Access in EDT - Changes: - all: https://git.openjdk.org/jdk/pull/19474/files - new: https://git.openjdk.org/jdk/pull/19474/files/d2f9256b..78b3e70f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19474=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19474=01-02 Stats: 9 lines in 1 file changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19474/head:pull/19474 PR: https://git.openjdk.org/jdk/pull/19474
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 10:51:29 GMT, Abhishek Kumar wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Non-client aread edge area modified, test format > > test/jdk/javax/swing/JMenu/TestUngrab.java line 81: > >> 79: robot.delay(1000); >> 80: Point point = menu.getLocationOnScreen(); >> 81: Dimension dim = menu.getSize(); > > Should it be accessed on EDT? Yes, updated - PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622344582
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 09:55:37 GMT, Alexander Zvegintsev wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Non-client aread edge area modified, test format > > src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1029: > >> 1027: // Also, non-client area includes the edges at left, right >> and botton of frame >> 1028: if ((p.y >= (frame.origin.y + contentRect.size.height)) || >> 1029: (p.x >= (frame.origin.x + contentRect.size.width)) || > > Suggestion: > > (p.x >= (frame.origin.x + contentRect.size.width - 3)) || > > > From my testing there is a 3 pixel dead zone at the right window border, when > the resize cursor is displayed, but `deliverNCMouseDown` is not called. > > Please check this on your system as well. Yes, there was a dead zone..Thanks for pointing it out...Updated.. - PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622344303
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is > resized from the lower right corner, then the Menu / JPopupmenu stays open > unlike in native osx apps like Notes, Mail etc.. > > This is because when LMouseButton is pressed on non-client area, the window > should get a UngrabEvent for it to close/cancel the popupmenu (as is done in > [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) > but it was seen that the mac AWTWindow code only recognizes the title-bar as > the non-client area so > [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) > is not called. > Fix is made to recognize the edges of the window as non-client area Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Non-client aread edge area modified, test format - Changes: - all: https://git.openjdk.org/jdk/pull/19474/files - new: https://git.openjdk.org/jdk/pull/19474/files/4b22432b..d2f9256b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19474=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19474=00-01 Stats: 17 lines in 2 files changed: 8 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/19474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19474/head:pull/19474 PR: https://git.openjdk.org/jdk/pull/19474
Re: RFR: 8280990: [XWayland] XTest emulated mouse click does not bring window to front
On Tue, 28 May 2024 02:25:46 GMT, Alexander Zvegintsev wrote: > Some of the modal tests fail in X11 compatibility mode on Wayland, because a > mouse click emulated with XTEST does not not cause the windows to be > reordered. > > This is a known limitation because these click events do not leave the > XWayland server and are not reported to the Wayland compositor. > > There is a [libei](https://gitlab.freedesktop.org/libinput/libei) that can be > used to emulate input events in Wayland. > And there is a [bridge that allows emulated events from XTEST to be passed to > the > compositor](https://gitlab.freedesktop.org/libinput/libei/-/blob/main/README.md?ref_type=heads#xwayland-and-xtest). > Support for this has been added in [Gnome 45 and XWayland > 23.2](https://gitlab.gnome.org/GNOME/mutter/-/issues/3194#note_1937109), but > it is optional and not yet enabled in Ubuntu. > > This change adds `toFront` calls for mouse clicks that are only intended to > bring a window to the front in the window stacking order. > > The testing is green on all platforms. WIll it be a problem if we use toFront for the frame anyways? We have seen and done tofront() for many tests where the frame gets obscured by some other window so mouseclicks was not on desired area - Marked as reviewed by psadhukhan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19417#pullrequestreview-2090670877
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Thu, 30 May 2024 20:37:51 GMT, Alexander Scherbatiy wrote: >> What I understood is, it is acceptable to identify or get >> tray-1, tray-2, tray-3 name >> instead of descriptive names like >> TOP, MIDDLE, BOTTOM respectively etc >> so as such it can support upto 11 trays ie FROM TOP till LARGE_CAPACITY >> >> but am not sure if it is to be added in addition or in-place of the >> descriptive names? > > It is easy to add new constants to a public API but there is no way to remove > them. > OutpuBin constants work only if they are listed among the supported output > bins. > There is always a way to lookup `tray-n` output bin through the supported > ones and it would work in the same way if it was included as a predefined > constant. If it's only about adding tray-1 to tray-11 in the above list and not about whole lot of changes in other areas, so that supported list contains those "tray-N" values, I guess we should add them.. I guess isSupportedAttributedValues(tray-N) will not return false in that case.. - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1621817791
RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized
Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is resized from the lower right corner, then the Menu / JPopupmenu stays open unlike in native osx apps like Notes, Mail etc.. This is because when LMouseButton is pressed on non-client area, the window should get a UngrabEvent for it to close/cancel the popupmenu (as is done in [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) but it was seen that the mac AWTWindow code only recognizes the title-bar as the non-client area so [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) is not called. Fix is made to recognize the edges of the window as non-client area - Commit messages: - Copyright - Copyright - Copyright - jcheck - jcheck - 8325435: [macos] Menu or JPopupMenu not closed when main window is resized Changes: https://git.openjdk.org/jdk/pull/19474/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19474=00 Issue: https://bugs.openjdk.org/browse/JDK-8325435 Stats: 110 lines in 2 files changed: 109 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19474/head:pull/19474 PR: https://git.openjdk.org/jdk/pull/19474
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v16]
On Wed, 29 May 2024 19:56:37 GMT, Alexander Scherbatiy wrote: >> The fix adds new public `OutputBin` print attribute class which allow to set >> a printer output bin in a `PrinterJob` class. The corresponding internal >> `CustomOutputBin` class is added as well. >> >> - Constants used in `OutputBin` class are based on [Internet Printing >> Protocol (IPP): “output-bin” attribute >> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf) >> document. >> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD >> `ppdFindOption(..., "OutputBin")` function to get supported output bins for >> the given printer on native level. >> - The fix propagates the `OutputBin` attribute from the printer job >> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS. >> >> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where >> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice): >> - Printer settings, None >> - Inner tray, INNERTRAY >> - Separator tray, SEPARATORTRAY >> - Finisher (face-down), Main >> >> if `Printer settings`, `Inner tray`, or `Finisher (face-down)` >> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is >> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If >> `Separator tray` is used a page is printed to the Separator tray. This is >> consistent with the printer behavior when a native print dialog is used from >> a native Preview app to print a document on macOS. > > Alexander Scherbatiy has updated the pull request incrementally with five > additional commits since the last revision: > > - Update imports order in CheckSupportedOutputBinsTest > - Use only bug id number in bug tag in tests > - Fix imports order > - Fix formatting in the for loop > - Update PrintService.isAttributeValueSupported() function for OutputBin > constants test/jdk/javax/print/attribute/CheckSupportedOutputBinsTest.java line 36: > 34: * @test > 35: * @bug 8314070 > 36: * @key printer DO you want to run this on all platforms? Other tests are for linux and mac only!! but guess it's not a problem.. - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1620133259
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Wed, 29 May 2024 20:20:00 GMT, Alexander Scherbatiy wrote: >> Actually it is not clear for me which maximum N should be used for these >> constants. >> Should only `tray-1`, `tray-2`, and `tray-3` be added, or should there be >> constants `tray-1` ... `tray-5` or even more? > > I updated the `PrintService.isAttributeValueSupported()` method that it > always returns true for CustomOutputBin class and returns true for an > OutputBin constant only if its original name (ipp case) or the name without > `-` sign (cups case) equals to one of the supported output bin choice name. What I understood is, it is acceptable to identify or get tray-1, tray-2, tray-3 name instead of descriptive names like TOP, MIDDLE, BOTTOM respectively etc so as such it can support upto 11 trays ie FROM TOP till LARGE_CAPACITY but am not sure if it is to be added in addition or in-place of the descriptive names? - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1619889088
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v15]
On Tue, 28 May 2024 16:47:21 GMT, Alexander Scherbatiy
wrote:
>> The fix adds new public `OutputBin` print attribute class which allow to set
>> a printer output bin in a `PrinterJob` class. The corresponding internal
>> `CustomOutputBin` class is added as well.
>>
>> - Constants used in `OutputBin` class are based on [Internet Printing
>> Protocol (IPP): “output-bin” attribute
>> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf)
>> document.
>> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD
>> `ppdFindOption(..., "OutputBin")` function to get supported output bins for
>> the given printer on native level.
>> - The fix propagates the `OutputBin` attribute from the printer job
>> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS.
>>
>> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where
>> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice):
>> - Printer settings, None
>> - Inner tray, INNERTRAY
>> - Separator tray, SEPARATORTRAY
>> - Finisher (face-down), Main
>>
>> if `Printer settings`, `Inner tray`, or `Finisher (face-down)`
>> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is
>> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If
>> `Separator tray` is used a page is printed to the Separator tray. This is
>> consistent with the printer behavior when a native print dialog is used from
>> a native Preview app to print a document on macOS.
>
> Alexander Scherbatiy has updated the pull request incrementally with four
> additional commits since the last revision:
>
> - Move OutputBin import before PageRanges in RasterPrinterJob
> - Add item listener to cbOutput only if the cbOutput is enabled
> - Add messages if no print service is found or OutputBin category is not
> supported
> - Move OutputBin import before PageRanges in CPrinterJob.java
src/java.desktop/share/classes/sun/print/PSPrinterJob.java line 1652:
> 1650: }
> 1651: if ((pFlags & OPTIONS) != 0) {
> 1652: for(String option: options.trim().split(" ")) {
code formatting..space between for and (
src/java.desktop/share/classes/sun/print/PSPrinterJob.java line 1677:
> 1675: }
> 1676: if ((pFlags & OPTIONS) != 0) {
> 1677: for(String option: options.trim().split(" ")) {
space is needed for coding style guideline between for and {
src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 42:
> 40: import javax.print.attribute.standard.MediaSize;
> 41: import javax.print.attribute.standard.MediaTray;
> 42: import javax.print.attribute.standard.OutputBin;
Import sorting improper...Should be after MediaPrintableArea and before
PrinterResolution
test/jdk/javax/print/attribute/CheckSupportedOutputBinsTest.java line 34:
> 32: /*
> 33: * @test
> 34: * @bug JDK-8314070
should be just bugid
test/jdk/javax/print/attribute/OutputBinAttributePrintDialogTest.java line 60:
> 58: /*
> 59: * @test
> 60: * @bug JDK-8314070
only bugid is needed
test/jdk/javax/print/attribute/OutputBinAttributeTest.java line 58:
> 56: /*
> 57: * @test
> 58: * @bug JDK-8314070
same here only bugid needed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618300643
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618301861
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618305622
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618309492
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618320383
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618323832
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Mon, 20 May 2024 06:48:15 GMT, Prasanta Sadhukhan wrote: >> Alexander Scherbatiy has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Make OutputBin class sealed > > src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java > line 154: > >> 152: FACE_UP, >> 153: FACE_DOWN, >> 154: LARGE_CAPACITY, > > What about this support from IPP doc? > >> ‘tray-N’: Output bins that are best identified as ‘tray-1’, ‘tray-2’, ... >> rather than the descriptive names >> defined in the above keyword list. >> What about this support? - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1618297028
Integrated: 8332431: NullPointerException in JTable of SwingSet2
On Fri, 24 May 2024 05:03:07 GMT, Prasanta Sadhukhan wrote: > Issue is observed in JTable demo in SwingSet2 whereby if we set the focus on > a table cell (or click on a table cell) and Press Ctrl+F1 (show/hide tooltip) > on a cell of JTable then NullpointerException is seen > >> Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: >> Cannot invoke "java.awt.event.MouseEvent.getLocationOnScreen()" because >> "this.mouseEvent" is null > > This is because ToolTip associated with JTable demo button at top of > SwingSet2 has MouseMotion Listener which causes [mouseEvent to be > null](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L573) > when mouse exits the demo button area and enter the table cells area where > show/hide tooltip causes showTipWindow to be called trying to access > `mouseEvent` which is null. > Fix is made to check for null in this kind of cases..No regression test is > added as it can be checked easily with SwingSet2.. This pull request has now been integrated. Changeset: 9a83dfee Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/9a83dfee14f4cd9cda476d11a027294a810953cb Stats: 16 lines in 1 file changed: 10 ins; 0 del; 6 mod 8332431: NullPointerException in JTable of SwingSet2 Reviewed-by: abhiscxk, kizune - PR: https://git.openjdk.org/jdk/pull/19379
Re: RFR: 8332431: NullPointerException in JTable of SwingSet2 [v3]
> Issue is observed in JTable demo in SwingSet2 whereby if we set the focus on > a table cell (or click on a table cell) and Press Ctrl+F1 (show/hide tooltip) > on a cell of JTable then NullpointerException is seen > >> Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: >> Cannot invoke "java.awt.event.MouseEvent.getLocationOnScreen()" because >> "this.mouseEvent" is null > > This is because ToolTip associated with JTable demo button at top of > SwingSet2 has MouseMotion Listener which causes [mouseEvent to be > null](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L573) > when mouse exits the demo button area and enter the table cells area where > show/hide tooltip causes showTipWindow to be called trying to access > `mouseEvent` which is null. > Fix is made to check for null in this kind of cases..No regression test is > added as it can be checked easily with SwingSet2.. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Copyright year update - Changes: - all: https://git.openjdk.org/jdk/pull/19379/files - new: https://git.openjdk.org/jdk/pull/19379/files/2e9c2337..ae3cde25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19379=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19379=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19379.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19379/head:pull/19379 PR: https://git.openjdk.org/jdk/pull/19379
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v14]
On Mon, 27 May 2024 17:02:35 GMT, Alexander Scherbatiy
wrote:
>> The fix adds new public `OutputBin` print attribute class which allow to set
>> a printer output bin in a `PrinterJob` class. The corresponding internal
>> `CustomOutputBin` class is added as well.
>>
>> - Constants used in `OutputBin` class are based on [Internet Printing
>> Protocol (IPP): “output-bin” attribute
>> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf)
>> document.
>> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD
>> `ppdFindOption(..., "OutputBin")` function to get supported output bins for
>> the given printer on native level.
>> - The fix propagates the `OutputBin` attribute from the printer job
>> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS.
>>
>> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where
>> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice):
>> - Printer settings, None
>> - Inner tray, INNERTRAY
>> - Separator tray, SEPARATORTRAY
>> - Finisher (face-down), Main
>>
>> if `Printer settings`, `Inner tray`, or `Finisher (face-down)`
>> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is
>> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If
>> `Separator tray` is used a page is printed to the Separator tray. This is
>> consistent with the printer behavior when a native print dialog is used from
>> a native Preview app to print a document on macOS.
>
> Alexander Scherbatiy has updated the pull request incrementally with four
> additional commits since the last revision:
>
> - Fix types in tests
> - Fix typos in tests
> - Check if OutputBin category supported in tests
> - Add automated CheckSupportedOutputBinsTest test
src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 93:
> 91: import javax.print.attribute.standard.SheetCollate;
> 92: import javax.print.attribute.standard.Sides;
> 93: import javax.print.attribute.standard.OutputBin;
Sort the import..OutputBin should be placed before PageRanges
src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 1283:
> 1281: if (!isSupportedValue(outputBinAttr, attributes)) {
> 1282: outputBinAttr = null;
> 1283: }
Should we set it to null if user-set output-bin attribute is not among
supported values
or
should we set it to default
as is done for printer-resolution and others
src/java.desktop/share/classes/sun/print/ServiceDialog.java line 2927:
> 2925: lblOutput.setEnabled(outputEnabled);
> 2926:
> 2927: cbOutput.addItemListener(this);
If Attribute category is unsupported, the panel will be disabled, in that case,
is there any need to add itemlistener? I guess we can make it conditional to
category being supported
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1616918563
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1616933045
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1616942526
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v13]
On Mon, 27 May 2024 16:59:08 GMT, Alexander Scherbatiy wrote: >> test/jdk/javax/print/attribute/OutputBinAttributePrintDialogTest.java line >> 187: >> >>> 185: return new OutputBin[0]; >>> 186: } >>> 187: >> >> I guess before getting supported Attribute Values, we need to ensure >> category is supported by calling >> `service.isAttributeCategorySupported(OutputBin.class)` >> in my opinion > > The test updated to check if the OutputBin category is supported. I think if `isATtributeCategorySupported `returns false, we should not return OutputBin[0] as that will cause the test to pass with message "SKip the test as number of supported outputbins is less than 2", which is misleading, in my opinion It should instead message "Skipping the test as OutputBin category is not supported for this printer" Also, `getPrintService` returning null also return the above outputbin message..It should rather return "No print service found" - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1616765093
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v14]
On Mon, 27 May 2024 17:02:35 GMT, Alexander Scherbatiy wrote: >> The fix adds new public `OutputBin` print attribute class which allow to set >> a printer output bin in a `PrinterJob` class. The corresponding internal >> `CustomOutputBin` class is added as well. >> >> - Constants used in `OutputBin` class are based on [Internet Printing >> Protocol (IPP): “output-bin” attribute >> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf) >> document. >> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD >> `ppdFindOption(..., "OutputBin")` function to get supported output bins for >> the given printer on native level. >> - The fix propagates the `OutputBin` attribute from the printer job >> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS. >> >> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where >> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice): >> - Printer settings, None >> - Inner tray, INNERTRAY >> - Separator tray, SEPARATORTRAY >> - Finisher (face-down), Main >> >> if `Printer settings`, `Inner tray`, or `Finisher (face-down)` >> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is >> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If >> `Separator tray` is used a page is printed to the Separator tray. This is >> consistent with the printer behavior when a native print dialog is used from >> a native Preview app to print a document on macOS. > > Alexander Scherbatiy has updated the pull request incrementally with four > additional commits since the last revision: > > - Fix types in tests > - Fix typos in tests > - Check if OutputBin category supported in tests > - Add automated CheckSupportedOutputBinsTest test src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java line 49: > 47: import javax.print.attribute.standard.PageRanges; > 48: import javax.print.attribute.standard.Sides; > 49: import javax.print.attribute.standard.OutputBin; Please sort the import...Guess OutputBin should come before PageRanges - PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1616767613
Re: RFR: 8332431: NullPointerException in JTable of SwingSet2 [v2]
On Tue, 28 May 2024 04:53:08 GMT, Abhishek Kumar wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Additional null check
>
> src/java.desktop/share/classes/javax/swing/ToolTipManager.java line 271:
>
>> 269: toFind = mouseEvent.getLocationOnScreen();
>> 270: } else {
>> 271: toFind = screenLocation;
>
> Why `toFind` is set to `screenLocation` ?
If mouse movement is not there, then it is imperative to use the original
screenlocation to find GraphicsConfiuration but as told this codepath was not
used so probably a no-op and should not be an issue..
> src/java.desktop/share/classes/javax/swing/ToolTipManager.java line 310:
>
>> 308: }
>> 309: } else {
>> 310: if (mouseEvent != null) {
>
> this may not be in the scope of changes but is it worth spending some time
> for code formatting ? Looks very messy.
may be as a cleanup later on, the whole file formatting is off at many places..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19379#discussion_r1616702383
PR Review Comment: https://git.openjdk.org/jdk/pull/19379#discussion_r1616703095
Re: RFR: 8332431: NullPointerException in JTable of SwingSet2 [v2]
On Mon, 27 May 2024 09:31:51 GMT, Abhishek Kumar wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Additional null check
>
> src/java.desktop/share/classes/javax/swing/ToolTipManager.java line 273:
>
>> 271: GraphicsConfiguration gc = getDrawingGC(toFind);
>> 272: if (gc == null) {
>> 273: if (mouseEvent != null) {
>
> In the same method **showTipWindow**, mouseEvent's _getLocationOnScreen_
> method is accessed at
> [L268](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L268C13-L268C13)
> and mouseEvent's _getX_ and _getY_ methods are accessed at
> [L304](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L304),
> guess we should guard them as well with null check.
I had seen it but not sure in which condition it will fail as it had not failed
until now in those codepaths..But Anyways, I agree and added the null check..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19379#discussion_r1616519362
Re: RFR: 8332431: NullPointerException in JTable of SwingSet2 [v2]
> Issue is observed in JTable demo in SwingSet2 whereby if we set the focus on > a table cell (or click on a table cell) and Press Ctrl+F1 (show/hide tooltip) > on a cell of JTable then NullpointerException is seen > >> Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: >> Cannot invoke "java.awt.event.MouseEvent.getLocationOnScreen()" because >> "this.mouseEvent" is null > > This is because ToolTip associated with JTable demo button at top of > SwingSet2 has MouseMotion Listener which causes [mouseEvent to be > null](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L573) > when mouse exits the demo button area and enter the table cells area where > show/hide tooltip causes showTipWindow to be called trying to access > `mouseEvent` which is null. > Fix is made to check for null in this kind of cases..No regression test is > added as it can be checked easily with SwingSet2.. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Additional null check - Changes: - all: https://git.openjdk.org/jdk/pull/19379/files - new: https://git.openjdk.org/jdk/pull/19379/files/00ac8e47..2e9c2337 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19379=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19379=00-01 Stats: 11 lines in 1 file changed: 8 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19379.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19379/head:pull/19379 PR: https://git.openjdk.org/jdk/pull/19379
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v13]
On Thu, 23 May 2024 05:03:35 GMT, Alexander Scherbatiy
wrote:
>> The fix adds new public `OutputBin` print attribute class which allow to set
>> a printer output bin in a `PrinterJob` class. The corresponding internal
>> `CustomOutputBin` class is added as well.
>>
>> - Constants used in `OutputBin` class are based on [Internet Printing
>> Protocol (IPP): “output-bin” attribute
>> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf)
>> document.
>> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD
>> `ppdFindOption(..., "OutputBin")` function to get supported output bins for
>> the given printer on native level.
>> - The fix propagates the `OutputBin` attribute from the printer job
>> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS.
>>
>> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where
>> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice):
>> - Printer settings, None
>> - Inner tray, INNERTRAY
>> - Separator tray, SEPARATORTRAY
>> - Finisher (face-down), Main
>>
>> if `Printer settings`, `Inner tray`, or `Finisher (face-down)`
>> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is
>> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If
>> `Separator tray` is used a page is printed to the Separator tray. This is
>> consistent with the printer behavior when a native print dialog is used from
>> a native Preview app to print a document on macOS.
>
> Alexander Scherbatiy has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Move jtreg tags below the imports in the tests
Previous comments about `output-bin-default` in
IPPPrintServive.getDefaultAttributeValues and `tray-N` in supported output-bins
are still pending..
test/jdk/javax/print/attribute/OutputBinAttributePrintDialogTest.java line 187:
> 185: return new OutputBin[0];
> 186: }
> 187:
I guess before getting supported Attribute Values, we need to ensure category
is supported by calling `service.isAttributeCategorySupported(OutputBin.class)`
in my opinion
test/jdk/javax/print/attribute/OutputBinAttributeTest.java line 165:
> 163: if (service == null) {
> 164: return supportedOutputBins;
> 165: }
same here for check of `isAttributeCategorySupported()`
-
PR Review: https://git.openjdk.org/jdk/pull/16166#pullrequestreview-2080211256
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1615612239
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1615613694
Re: RFR: 8329667: [macos] Issue with JTree related fix for JDK-8317771 [v4]
On Wed, 22 May 2024 06:38:26 GMT, Alexander Zuev wrote: >> Caching children and selected children of the thee on the native level; >> Caching all children of a specific parent in CAccessibility to enhance >> recursive children selection algorithm; >> Removing fix for JDK-8317771 as no longer needed; > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Replaced NULL with nil in OutlineAccessibility - functionally it has no > effect but it is more uiform and according to the ObjectibeC convention where > objects are referred as nil and references to the primitives as NULL. I hope you have ran the whole gamut of jtreg tests, if not, please do it before integrating and SwingSet2 too.. - Marked as reviewed by psadhukhan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19255#pullrequestreview-2076566438
Re: RFR: 8332416: Add more font selection options to Font2DTest [v3]
On Tue, 21 May 2024 23:08:17 GMT, Phil Race wrote: >> Enhance Font2DTest as follows >> >> - Add main menu Radio Button options so that you select the font to use as >> either >> - (1) Font Family + Style (as now) >> - (2) Font Family + Menu of all members of the Family, replacing the Style >> - (3) List of all fontnames - which can still be adjusted by Style if you >> want. >> The default is (1) so nothing looks different except that I updated the UI >> to use Nimbus instead of Metal. >> >> There's new code to gather these ways of referencing the fonts. >> Also changes were needed for the "Save/Load" options to include the new UI >> state and font settings. > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8332416 src/demo/share/jfc/Font2DTest/Font2DTest.java line 1443: > 1441: }); > 1442: > 1443: f.getContentPane().add( f2dt ); Since this is new code, we can remove getContentPane which is not needed for add - PR Review Comment: https://git.openjdk.org/jdk/pull/19273#discussion_r1612815044
Re: RFR: 8332416: Add more font selection options to Font2DTest [v3]
On Tue, 21 May 2024 23:08:17 GMT, Phil Race wrote:
>> Enhance Font2DTest as follows
>>
>> - Add main menu Radio Button options so that you select the font to use as
>> either
>> - (1) Font Family + Style (as now)
>> - (2) Font Family + Menu of all members of the Family, replacing the Style
>> - (3) List of all fontnames - which can still be adjusted by Style if you
>> want.
>> The default is (1) so nothing looks different except that I updated the UI
>> to use Nimbus instead of Metal.
>>
>> There's new code to gather these ways of referencing the fonts.
>> Also changes were needed for the "Save/Load" options to include the new UI
>> state and font settings.
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8332416
src/demo/share/jfc/Font2DTest/FontPanel.java line 742:
> 740: verticalBar.setValues( oldValue, numCharDown, 0,
> totalNumRows );
> 741: }
> 742: if ( totalNumRows <= numCharDown && drawStart == 0) {
Is this check needed now since both if and else condition do the same thing?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19273#discussion_r1612809269
RFR: 8332431: NullPointerException in JTable of SwingSet2
Issue is observed in JTable demo in SwingSet2 whereby if we set the focus on a table cell (or click on a table cell) and Press Ctrl+F1 (show/hide tooltip) on a cell of JTable then NullpointerException is seen > Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: Cannot > invoke "java.awt.event.MouseEvent.getLocationOnScreen()" because > "this.mouseEvent" is null This is because ToolTip associated with JTable demo button at top of SwingSet2 has MouseMotion Listener which causes [mouseEvent to be null](https://github.com/openjdk/jdk/blame/da3001daf79bf943d6194d9fd60250d519b9680d/src/java.desktop/share/classes/javax/swing/ToolTipManager.java#L573) when mouse exits the demo button area and enter the table cells area where show/hide tooltip causes showTipWindow to be called trying to access `mouseEvent` which is null. Fix is made to check for null in this kind of cases..No regression test is added as it can be checked easily with SwingSet2.. - Commit messages: - 8332431: NullPointerException in JTable of SwingSet2 Changes: https://git.openjdk.org/jdk/pull/19379/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19379=00 Issue: https://bugs.openjdk.org/browse/JDK-8332431 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19379.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19379/head:pull/19379 PR: https://git.openjdk.org/jdk/pull/19379
Integrated: 8307193: Several Swing jtreg tests use class.forName on L classes
On Wed, 22 May 2024 05:28:01 GMT, Prasanta Sadhukhan wrote: > Opensource couple RTL tests This pull request has now been integrated. Changeset: 9d332e65 Author: Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/9d332e6591334a71335da65a4dd7b2ed0482b6cb Stats: 326 lines in 2 files changed: 326 ins; 0 del; 0 mod 8307193: Several Swing jtreg tests use class.forName on L classes Reviewed-by: abhiscxk, prr - PR: https://git.openjdk.org/jdk/pull/19341
Re: RFR: 8307193: Several Swing jtreg tests use class.forName on L classes [v2]
> Opensource couple RTL tests Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: spacing - Changes: - all: https://git.openjdk.org/jdk/pull/19341/files - new: https://git.openjdk.org/jdk/pull/19341/files/e9c1da0e..9f26b8f0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19341=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19341=00-01 Stats: 6 lines in 2 files changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19341.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19341/head:pull/19341 PR: https://git.openjdk.org/jdk/pull/19341
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v10]
On Tue, 21 May 2024 19:01:35 GMT, Alexander Scherbatiy wrote: >> The fix adds new public `OutputBin` print attribute class which allow to set >> a printer output bin in a `PrinterJob` class. The corresponding internal >> `CustomOutputBin` class is added as well. >> >> - Constants used in `OutputBin` class are based on [Internet Printing >> Protocol (IPP): “output-bin” attribute >> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf) >> document. >> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD >> `ppdFindOption(..., "OutputBin")` function to get supported output bins for >> the given printer on native level. >> - The fix propagates the `OutputBin` attribute from the printer job >> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS. >> >> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where >> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice): >> - Printer settings, None >> - Inner tray, INNERTRAY >> - Separator tray, SEPARATORTRAY >> - Finisher (face-down), Main >> >> if `Printer settings`, `Inner tray`, or `Finisher (face-down)` >> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is >> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If >> `Separator tray` is used a page is printed to the Separator tray. This is >> consistent with the printer behavior when a native print dialog is used from >> a native Preview app to print a document on macOS. > > Alexander Scherbatiy has updated the pull request incrementally with one > additional commit since the last revision: > > Add OutputBin support on Linux I am trying to test on "Xerox AltaLink B8155" MFP printer which is installed as IPP protocol to find "Generic Postscript Printer" driver..Selecting "AirPrint" is not able to find any driver for my macos14.3..and I am not able to find any "Finishing Options" in my native dialog as shown below so unable to test.. https://github.com/openjdk/jdk/assets/43534309/abe22bd7-e6d6-4c54-888d-c1fa70f76aad;> Also, I thought the consensus was to staggard the fix and let one platform get in for it to bake for sometime and then add another platform later on.. - PR Comment: https://git.openjdk.org/jdk/pull/16166#issuecomment-2123918666
RFR: 8307193: Several Swing jtreg tests use class.forName on L classes
Opensource couple RTL tests - Commit messages: - 8307193: Several Swing jtreg tests use class.forName on L classes Changes: https://git.openjdk.org/jdk/pull/19341/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19341=00 Issue: https://bugs.openjdk.org/browse/JDK-8307193 Stats: 328 lines in 2 files changed: 328 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19341.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19341/head:pull/19341 PR: https://git.openjdk.org/jdk/pull/19341
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Mon, 20 May 2024 23:29:53 GMT, Alisen Chung wrote:
> Are the tests added supposed to be manual tests? I’m not seeing any dialogs
> pop up when i run the test on jtreg. Maybe I missed a config setting? I'm
> running the tests on mac.
I guess its because of this code where supported output bins is < 1 which
prevents printdialog to popup and the test Passed or rather did not
proceed..Not sure if this should be considered "Passed" or "Skipped"
```
OutputBin[] supportedOutputBins = getSupportedOutputBinttributes();
if (supportedOutputBins.length < 1) {
return;
}
Also, in my native print dialog also, I am not seeing any "output bin" setting.
Where exactly it will be shown?
https://github.com/openjdk/jdk/assets/43534309/687d66f6-b8da-496c-92ae-32f63068962b;>
-
PR Comment: https://git.openjdk.org/jdk/pull/16166#issuecomment-2122557975
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v9]
On Mon, 20 May 2024 08:45:15 GMT, Abhishek Kumar wrote: >> JLabel text is not painted with the LAF defined foreground color in GTK LAF. >> In GTK LAF the foreground color is retrieved by using native system APIs. >> Fix is to return the foreground color if it is set by LAF defined property >> otherwise return the default color by calling native APIs. >> Applet based test has been converted to automatic test and check for all >> installed LAFs. CI testing is green for test suite and individual test. Link >> attached in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > jtreg bug id tag update Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17763#pullrequestreview-2065699944
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Sat, 11 May 2024 17:32:37 GMT, Alexander Scherbatiy
wrote:
>> The fix adds new public `OutputBin` print attribute class which allow to set
>> a printer output bin in a `PrinterJob` class. The corresponding internal
>> `CustomOutputBin` class is added as well.
>>
>> - Constants used in `OutputBin` class are based on [Internet Printing
>> Protocol (IPP): “output-bin” attribute
>> extension](https://ftp.pwg.org/pub/pwg/candidates/cs-ippoutputbin10-20010207-5100.2.pdf)
>> document.
>> - `CUPSPrinter.getOutputBins(String printer)` method uses PPD
>> `ppdFindOption(..., "OutputBin")` function to get supported output bins for
>> the given printer on native level.
>> - The fix propagates the `OutputBin` attribute from the printer job
>> attributes to `NSPrintInfo` print settings with `OutputBin` key on macOS.
>>
>> The fix was tested on `Kyocera ECOSYS M8130cidn` printer where
>> `ppdFindOption(..., "OutputBin")` call returns 4 output bins (text, choice):
>> - Printer settings, None
>> - Inner tray, INNERTRAY
>> - Separator tray, SEPARATORTRAY
>> - Finisher (face-down), Main
>>
>> if `Printer settings`, `Inner tray`, or `Finisher (face-down)`
>> CustomOutputBins is set to `PrinterJob.print(...)` attributes a test page is
>> printed to the Main tray of the `Kyocera ECOSYS M8130cidn` printer. If
>> `Separator tray` is used a page is printed to the Separator tray. This is
>> consistent with the printer behavior when a native print dialog is used from
>> a native Preview app to print a document on macOS.
>
> Alexander Scherbatiy has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Make OutputBin class sealed
src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java
line 46:
> 44: *
> 45: * IPP Compatibility: This attribute is not an IPP 1.1 attribute;
> it is
> 46: * an attribute in the "output-bin" attribute extension
Are we also supporting "output-destination" attribute to specify printing to a
destination as per the IPP doc
>
> Since the destination may also
> be electronic and have a method associated with it, also allow the uri
> attribute syntax. Probably call this
> other attribute “output-destination” with an attribute syntax of (1setOf uri
> | name). Or possibly the output-
> destination should be a parameter on the URL?
>
src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java
line 137:
> 135: "face-up",
> 136: "face-down",
> 137: "large-capacity",
Are we considering this from IPP doc for the internationalization of this
keywords?
> Normally a client will provide localization of the keywords values of this
> attribute to the language of the
> user,
src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java
line 154:
> 152: FACE_UP,
> 153: FACE_DOWN,
> 154: LARGE_CAPACITY,
What about this support from IPP doc?
> ‘tray-N’: Output bins that are best identified as ‘tray-1’, ‘tray-2’, ...
> rather than the descriptive names
> defined in the above keyword list.
>
src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java
line 184:
> 182: */
> 183: @Override
> 184: public final Class getCategory() {
It does not seem this PR supports
`output-bin-default` mentioned in the IPP doc, which will be returned via
[getDefaultAttributeValue](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/PrintService.html#getDefaultAttributeValue(java.lang.Class))([Class](https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/Class.html)https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/attribute/Attribute.html)>
category)?
If not, what this API will return?
src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 65:
> 63: private static synchronized native String[] getMedia(String printer);
> 64: private static synchronized native float[] getPageSizes(String
> printer);
> 65: private static synchronized native String[] getOutputBins(String
> printer);
Will it return all the output-bins or only the supported ones? Iguess it's the
latter, then I think it's better to rename it as "getSupportedOutputBins" as
this follows "“output-bin-supported” attribute of IPP document..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606307018
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606328005
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606325741
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606310284
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606302826
Re: RFR: 8314070: javax.print: Support IPP output-bin attribute extension [v9]
On Mon, 20 May 2024 06:30:06 GMT, Prasanta Sadhukhan
wrote:
>> Alexander Scherbatiy has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Make OutputBin class sealed
>
> src/java.desktop/share/classes/javax/print/attribute/standard/OutputBin.java
> line 184:
>
>> 182: */
>> 183: @Override
>> 184: public final Class getCategory() {
>
> It does not seem this PR supports
> `output-bin-default` mentioned in the IPP doc, which will be returned via
> [getDefaultAttributeValue](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/PrintService.html#getDefaultAttributeValue(java.lang.Class))([Class](https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/Class.html) extends
> [Attribute](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/attribute/Attribute.html)>
> category)?
> If not, what this API will return?
Also, as per IPP document
>
> When returning the values of the
> associated “output-bin-supported” attribute, the values returned MAY depend
> on the user issuing the Get-
> Printer-Attributes operation
>
I think, we are supposed to return the supported output-bins when user calls
[getSupportedAttributeValues](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/PrintService.html#getSupportedAttributeValues(java.lang.Class,javax.print.DocFlavor,javax.print.attribute.AttributeSet))([Class](https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/Class.html)https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/attribute/Attribute.html)>
category,
[DocFlavor](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/DocFlavor.html)
flavor,
[AttributeSet](https://docs.oracle.com/en/java/javase/22/docs/api/java.desktop/javax/print/attribute/AttributeSet.html)
attributes)
which it seems is not done..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16166#discussion_r1606324503
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v5]
On Tue, 5 Mar 2024 08:20:12 GMT, Prasanta Sadhukhan wrote: >> Abhishek Kumar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> separate method to get LAF defined color > > test/jdk/javax/swing/plaf/basic/BasicHTML/bug4248210.java line 40: > >> 38: /* >> 39: * @test >> 40: * @bug 4248210 8075917 4314194 > > 4314194 remove from here.. I guess both 8075917 4314194 needs to be removed as this PR is not having any product fix and this test is just being opensourced.. - PR Review Comment: https://git.openjdk.org/jdk/pull/17763#discussion_r1606356880
Integrated: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs
On Fri, 17 May 2024 03:37:21 GMT, Prasanta Sadhukhan wrote: > Inadvertent mention of Netscape in Javadoc is removed.. This pull request has now been integrated. Changeset: b92bd671 Author: Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/b92bd671835c37cff58e2cdcecd0fe4277557d7f Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod 8332403: Anachronistic reference to Netscape Communicator in Swing API docs Reviewed-by: abhiscxk, aivanov, prr - PR: https://git.openjdk.org/jdk/pull/19276
Re: RFR: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs [v3]
> Inadvertent mention of Netscape in Javadoc is removed.. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: e.g typo - Changes: - all: https://git.openjdk.org/jdk/pull/19276/files - new: https://git.openjdk.org/jdk/pull/19276/files/fed045f8..6c06e49d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19276=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19276=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19276.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19276/head:pull/19276 PR: https://git.openjdk.org/jdk/pull/19276
Re: RFR: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs [v2]
> Inadvertent mention of Netscape in Javadoc is removed.. Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision: - doc clarity - doc clarity - Changes: - all: https://git.openjdk.org/jdk/pull/19276/files - new: https://git.openjdk.org/jdk/pull/19276/files/310635fc..fed045f8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19276=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19276=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19276.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19276/head:pull/19276 PR: https://git.openjdk.org/jdk/pull/19276
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v6]
On Thu, 16 May 2024 08:48:30 GMT, Abhishek Kumar wrote: >> JLabel text is not painted with the LAF defined foreground color in GTK LAF. >> In GTK LAF the foreground color is retrieved by using native system APIs. >> Fix is to return the foreground color if it is set by LAF defined property >> otherwise return the default color by calling native APIs. >> Applet based test has been converted to automatic test and check for all >> installed LAFs. CI testing is green for test suite and individual test. Link >> attached in JBS. > > Abhishek Kumar has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains seven additional > commits since the last revision: > > - Revert back the product fix and update test to skip GTK L > - Merge > - separate method to get LAF defined color > - extended fix for disabled checkbox and radiobutton > - Headful tag revert back and condition check for nimbus > - jtreg tag headful removed > - JLable foreground color fix for GTK LAF test/jdk/ProblemList.txt line 673: > 671: javax/swing/dnd/8139050/NativeErrorsInTableDnD.java 8202765 > macosx-all,linux-all > 672: javax/swing/JEditorPane/6917744/bug6917744.java 8213124 macosx-all > 673: javax/swing/JRadioButton/4314194/bug4314194.java 8298153 linux-all 8298153 needs to be closed then if this test is going to be deproblemlisted or add that JBS issue in this PR - PR Review Comment: https://git.openjdk.org/jdk/pull/17763#discussion_r1604470176
RFR: 8332403: Anachronistic reference to Netscape Communicator in Swing API docs
Inadvertent mention of Netscape in Javadoc is removed.. - Commit messages: - 8332403: Anachronistic reference to Netscape Communicator in Swing API docs - 8332403: Anachronistic reference to Netscape Communicator in Swing API docs Changes: https://git.openjdk.org/jdk/pull/19276/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19276=00 Issue: https://bugs.openjdk.org/browse/JDK-8332403 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19276.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19276/head:pull/19276 PR: https://git.openjdk.org/jdk/pull/19276
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v15]
On Fri, 3 May 2024 07:46:07 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Old test bug id added test/jdk/javax/swing/JTabbedPane/TestJTabbedPaneOpaqueColor.java line 43: > 41: /* > 42: * @test > 43: * @bug 8226990 6462396 4690946 Ideally it should be in first-to-last fix order sequence.. - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1588893403
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v14]
On Fri, 3 May 2024 05:10:15 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment update Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17720#pullrequestreview-2037455780
Re: RFR: 8233177: Remove testcase for JDK-8001470 as fix has been reverted [v3]
On Sat, 13 Apr 2024 18:22:07 GMT, Phil Race wrote: > Where are we with this ? Is there a consensus yet ? I guess I have added my explanations in the PR as to why the test should be removed...You can take a call and provide your decision. We have several unresolved bugs in JDK and most, if not all, have reproducible testcase. We dont add those testcase in the repo and problemlist it till the issue is resolved, right? Similarly, I think this testcase should be removed till we have the fix ready and a new JBS can be created with this removed testcase as reproducible test.. - PR Comment: https://git.openjdk.org/jdk/pull/17528#issuecomment-2092266578
Integrated: 8329559: Test javax/swing/JFrame/bug4419914.java failed because The End and Start buttons are not placed correctly and Tab focus does not move as expected
On Thu, 4 Apr 2024 05:34:18 GMT, Prasanta Sadhukhan wrote: > Test issue shows the End and Start buttons are not placed as per instructions > due to ComponentOrientation RTL was not honoured, as with `getContentPane()` > being removed from `add` call of JFrame, it was also additionally removed > from other JFrame calls which resulted in the RTL not being propagated to its > child. Fix is to add `getContentPane()` to the non-add methods of JFrame as > was done in the applet version of the test.. This pull request has now been integrated. Changeset: 7c1fad4f Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/7c1fad4fb6c387bbfb72b3f96b610e7cbc2ef312 Stats: 8 lines in 1 file changed: 2 ins; 1 del; 5 mod 8329559: Test javax/swing/JFrame/bug4419914.java failed because The End and Start buttons are not placed correctly and Tab focus does not move as expected Reviewed-by: abhiscxk, honkar, dnguyen - PR: https://git.openjdk.org/jdk/pull/18612
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v11]
On Thu, 2 May 2024 06:34:52 GMT, Abhishek Kumar wrote: >> src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf line 86: >> >>> 84: >> hueOffset="0.0" saturationOffset="0.0" brightnessOffset="0.0" >>> alphaOffset="0"/> >>> 85: >>> 86: >> >> One more thing >> All the above properties are prepended with nimbus except this so I guess it >> needs to be "nimbusTabbedPaneContentArea" > > There are properties without nimbus prepended as well... e.g. > textInactiveText, activeCaption etc. > Anyways Updated with the suggested name. ok..In that case, this property should have been put after those and not in between nimbus prepended properties... It's upto you to keep this or revert and move to appropriate location.. - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1587158233
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v12]
On Thu, 2 May 2024 06:40:20 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Nimbus property name update Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17720#pullrequestreview-2034976670
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v11]
On Thu, 2 May 2024 06:05:08 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Instruction and jtreg tag update src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf line 86: > 84: hueOffset="0.0" saturationOffset="0.0" brightnessOffset="0.0" > alphaOffset="0"/> > 85: > 86: One more thing All the above properties are prepended with nimbus except this so I guess it needs to be "nimbusTabbedPaneContentArea" - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1587117063
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v10]
On Fri, 19 Apr 2024 05:10:15 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > instruction and copyright year update test/jdk/javax/swing/JTabbedPane/TestJTabbedPaneOpaqueColor.java line 63: > 61: Check the default behaviour of the tabbed pane: > 62: - the area behind tabs is transparent (it must be green). > 63: - the tabs area is opaque (it must be red, except the > selected tab which must be gray). As per my testing in windows, this is not satisfied for Nimbus and all tabs are gray, not only the selected one.. Is this a bug not solved yet? Although you specify down below that tabs color are specific to nimbus style, this is prone to misinterpretation and can cause further issue..If it cannot be satisfied for Nimbus, it needs to be specified here upfront... test/jdk/javax/swing/JTabbedPane/TestJTabbedPaneOpaqueColor.java line 78: > 76: - the content area is opaque (it must be gray). > 77: > 78: Check this behaviour for other LAFs and tab layout. Since we check for only selected L like Metal/Nimbus/GTK/Aqua it is probably better to mention to check behaviour by clicking on present L button and tab layout.. Also, since this test is for all platforms, did you test on mac? test/jdk/javax/swing/JTabbedPane/TestJTabbedPaneOpaqueColor.java line 99: > 97: - the tabs are opaque (it must be red, except the selected > tab which must be gray). > 98: when unchecked: > 99: - the tabs are transparent (it must be gree). gree->green - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1584257458 PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1584537667 PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1584257656
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in Nimbus and Aqua LookAndFeel [v3]
On Wed, 10 Apr 2024 04:43:29 GMT, Tejesh R wrote: >> Fix suggested in bug >> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) >> had a regression in Nimbus L yet it resolved the issue in other L The >> better approach would be to handle `MultiResolutionImages `in `PathGraphics` >> class `getBufferedImage` method and is suggested here. The fix doesn't cause >> any regression and is verified in CI system. The test >> javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms >> and all L (Except in Windows it doesn't work due an issue >> ([JDK-8322135](https://bugs.openjdk.org/browse/JDK-8322135)) introduced in >> 22, yet to investigate on it). And also fix >> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) >> has been reverted, retaining the test. > > Tejesh R has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Copywrite year updated > - Merge branch 'master' of https://git.openjdk.java.net/jdk into > branch_8322140 > - Spacing updates > - Updated test with BugID and copyright year > - Fix + Revert 8210807 Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18187#pullrequestreview-1990795930
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel [v2]
On Thu, 4 Apr 2024 10:35:22 GMT, Tejesh R wrote: >> Fix suggested in bug >> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) >> had a regression in Nimbus L yet it resolved the issue in other L The >> better approach would be to handle `MultiResolutionImages `in `PathGraphics` >> class `getBufferedImage` method and is suggested here. The fix doesn't cause >> any regression and is verified in CI system. The test >> javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms >> and all L (Except in Windows it doesn't work due an issue >> ([JDK-8322135](https://bugs.openjdk.org/browse/JDK-8322135)) introduced in >> 22, yet to investigate on it). And also fix >> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) >> has been reverted, retaining the test. > > Tejesh R has updated the pull request incrementally with two additional > commits since the last revision: > > - Spacing updates > - Updated test with BugID and copyright year test/jdk/javax/swing/JTable/JTableScrollPrintTest.java line 2: > 1: /* > 2: * Copyright (c) 2023, 2014, Oracle and/or its affiliates. All rights > reserved. should be 2024 - PR Review Comment: https://git.openjdk.org/jdk/pull/18187#discussion_r132274
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v7]
On Thu, 4 Apr 2024 12:28:36 GMT, Abhishek Kumar wrote: >> src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf line 21653: >> >>> 21651:>> value="true"/> >>> 21652:>> value="true"/> >>> 21653: >> >> SInce you are introducing those 3 property in NimbusLookANdFeel, is this >> defines needed in skin.laf? > > I am not sure about this. Should I add them in skin.laf ? you have already added...i am asking it it's needed? - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1552104159
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v7]
On Thu, 4 Apr 2024 06:55:29 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment update src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java line 1026: > 1024: "TabbedPane.selected", tabbedPaneBg, > 1025: "TabbedPane.contentOpaque", Boolean.TRUE, > 1026: "TabbedPane.contentAreaColor", tabbedPaneBg, `TabbedPane.contentOpaque` and `TabbedPane.tabsOpaque` seems to go hand-in-hand and there's another issue for it https://bugs.openjdk.org/browse/JDK-6462396 so if we are supporting one and not other, it may not have desired effect.. - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1552103661
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
On Thu, 4 Apr 2024 14:43:43 GMT, Tejesh R wrote: > > > > > > Can you please explain why you need to handle MultiResolutionImage > > > > > > for this printing issue for NimbusL and why was it not needed for > > > > > > other L Also, you need to add this bugid to the test > > > > > > > > > > > > > > > The fix is for L other than Nimbus. It is working in Nimbus since > > > > > Image drawing is handled by SunGraphics2D class in > > > > > [drawHIDPIImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java#L3126) > > > > > method. Whereas other L, pathGraphics handles ImageDrawing where > > > > > MultiResolutionImage is not yet handled. The fix which I proposed for > > > > > [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) > > > > > bug fixed for Non-Nimbus L but caused regression for Nimbus. Hence > > > > > after further analysis and study the root cause was found out to be > > > > > Non-handling of MultiResolutionImage in > > > > > [getBufferedImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/print/PathGraphics.java#L1122). > > > > > > > > > > > > I guess the PathGraphics path should be used for printing images and > > > > should be common for all so why Nimbus is going via SunGraphics2D? [and > > > > it seems you updated the summary to include Aqua so Aqua is also going > > > > via SunGraphics2D?] > > > > > > > > > Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And > > > regarding why SunGraphics2D is used for Nimbus and Aqua, its because in > > > Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in > > > other its PW/WPathGraphics. I'm not sure why this is different. > > > > > > Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the > > subsequent code from where is starts to bifurcate in other L meaning > > where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for > > others? > > Yeah, it is from RasterPrinterJob class > [here](https://github.com/openjdk/jdk/blob/21867c929a2f2c961148f2cd1e79d672ac278d27/src/java.desktop/share/classes/sun/print/RasterPrinterJob.java#L2298). > And that is because of nonSolidColors present in Nimbus and Aqua L I didn't get how it creates PeekGraphics for Nimbus/Aqua and PathGraphics for others? I guess RasterPrinterJob will delegate to WPrinterJob in windows and PSPrinterJob in unix and it is not dependant on L - PR Comment: https://git.openjdk.org/jdk/pull/18187#issuecomment-2037768402
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v7]
On Thu, 4 Apr 2024 06:55:29 GMT, Abhishek Kumar wrote: >> JTabbedPane's content area, tab area and tab background color are not as >> expected when opaque is set to true or false. >> The proposed fix is to handle the TabbedPane's background color in installed >> LAFs. Manual test is added to support the fix and there is no regression >> caused by the fix. >> >> Proposed fix is tested in Ubuntu 22.04 and Oracle linux. >> >> CI link is posted in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment update src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf line 21653: > 21651: value="true"/> > 21652: value="true"/> > 21653: SInce you are introducing those 3 property in NimbusLookANdFeel, is this defines needed in skin.laf? - PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1551543921
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
On Thu, 4 Apr 2024 11:30:14 GMT, Tejesh R wrote: > > > > Can you please explain why you need to handle MultiResolutionImage for > > > > this printing issue for NimbusL and why was it not needed for other > > > > L Also, you need to add this bugid to the test > > > > > > > > > The fix is for L other than Nimbus. It is working in Nimbus since Image > > > drawing is handled by SunGraphics2D class in > > > [drawHIDPIImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java#L3126) > > > method. Whereas other L, pathGraphics handles ImageDrawing where > > > MultiResolutionImage is not yet handled. The fix which I proposed for > > > [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) > > > bug fixed for Non-Nimbus L but caused regression for Nimbus. Hence > > > after further analysis and study the root cause was found out to be > > > Non-handling of MultiResolutionImage in > > > [getBufferedImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/print/PathGraphics.java#L1122). > > > > > > I guess the PathGraphics path should be used for printing images and should > > be common for all so why Nimbus is going via SunGraphics2D? [and it seems > > you updated the summary to include Aqua so Aqua is also going via > > SunGraphics2D?] > > Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And > regarding why SunGraphics2D is used for Nimbus and Aqua, its because in > Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other > its PW/WPathGraphics. I'm not sure why this is different. Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the subsequent code from where is starts to bifurcate in other L meaning where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for others? - PR Comment: https://git.openjdk.org/jdk/pull/18187#issuecomment-2036938917
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
On Thu, 4 Apr 2024 09:55:27 GMT, Tejesh R wrote: > > Can you please explain why you need to handle MultiResolutionImage for this > > printing issue for NimbusL and why was it not needed for other L Also, > > you need to add this bugid to the test > > The fix is for L other than Nimbus. It is working in Nimbus since Image > drawing is handled by SunGraphics2D class in > [drawHIDPIImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java#L3126) > method. Whereas other L, pathGraphics handles ImageDrawing where > MultiResolutionImage is not yet handled. The fix which I proposed for > [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) > bug fixed for Non-Nimbus L but caused regression for Nimbus. Hence after > further analysis and study the root cause was found out to be Non-handling of > MultiResolutionImage in > [getBufferedImage()](https://github.com/openjdk/jdk/blob/b9da14012da5f1f72d4f6e690c18a43e87523173/src/java.desktop/share/classes/sun/print/PathGraphics.java#L1122). I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?] - PR Comment: https://git.openjdk.org/jdk/pull/18187#issuecomment-2036881285
Re: RFR: 8329559: Test javax/swing/JFrame/bug4419914.java failed because The End and Start buttons are not placed correctly and Tab focus does not move as expected [v2]
> Test issue shows the End and Start buttons are not placed as per instructions > due to ComponentOrientation RTL was not honoured, as with `getContentPane()` > being removed from `add` call of JFrame, it was also additionally removed > from other JFrame calls which resulted in the RTL not being propagated to its > child. Fix is to add `getContentPane()` to the non-add methods of JFrame as > was done in the applet version of the test.. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: instruction formatting - Changes: - all: https://git.openjdk.org/jdk/pull/18612/files - new: https://git.openjdk.org/jdk/pull/18612/files/61a0a049..79123987 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18612=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18612=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18612.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18612/head:pull/18612 PR: https://git.openjdk.org/jdk/pull/18612
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
On Mon, 11 Mar 2024 10:05:41 GMT, Tejesh R wrote: > Fix suggested in bug > [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) > had a regression in Nimbus L yet it resolved the issue in other L The > better approach would be to handle `MultiResolutionImages `in `PathGraphics` > class `getBufferedImage` method and is suggested here. The fix doesn't cause > any regression and is verified in CI system. The test > javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms > and all L (Except in Windows it doesn't work due an issue introduced in 22, > yet to investigate on it). And also fix > [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720) > has been reverted, retaining the test. Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL and why was it not needed for other L Also, you need to add this bugid to the test - PR Comment: https://git.openjdk.org/jdk/pull/18187#issuecomment-2036619933
RFR: 8329559: Test javax/swing/JFrame/bug4419914.java failed because The End and Start buttons are not placed correctly and Tab focus does not move as expected
Test issue shows the End and Start buttons are not placed as per instructions due to ComponentOrientation RTL was not honoured, as with `getContentPane()` being removed from `add` call of JFrame, it was also additionally removed from other JFrame calls which resulted in the RTL not being propagated to its child. Fix is to add `getContentPane()` to the non-add methods of JFrame as was done in the applet version of the test.. - Commit messages: - 8329559: Test javax/swing/JFrame/bug4419914.java failed because The End and Start buttons are not placed correctly and Tab focus does not move as expected Changes: https://git.openjdk.org/jdk/pull/18612/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18612=00 Issue: https://bugs.openjdk.org/browse/JDK-8329559 Stats: 6 lines in 1 file changed: 2 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18612.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18612/head:pull/18612 PR: https://git.openjdk.org/jdk/pull/18612
Re: RFR: 8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
On Mon, 11 Mar 2024 10:05:41 GMT, Tejesh R wrote:
> Fix suggested in bug
> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720)
> had a regression in Nimbus L yet it resolved the issue in other L The
> better approach would be to handle `MultiResolutionImages `in `PathGraphics`
> class `getBufferedImage` method and is suggested here. The fix doesn't cause
> any regression and is verified in CI system. The test
> javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms
> and all L (Except in Windows it doesn't work due an issue introduced in 22,
> yet to investigate on it). And also fix
> [8210807](https://github.com/openjdk/jdk/commit/38bbbe7588c94d3a0edd1c120ba49cbd0851a720)
> has been reverted, retaining the test.
guess we need a testcase even if manual
src/java.desktop/share/classes/sun/print/PathGraphics.java line 1137:
> 1135: return ((VolatileImage)img).getSnapshot();
> 1136: } else if (img instanceof MultiResolutionImage) {
> 1137: return convertToBufferedImage((MultiResolutionImage) img,
guess this casting is not needed..
src/java.desktop/share/classes/sun/print/PathGraphics.java line 1138:
> 1136: } else if (img instanceof MultiResolutionImage) {
> 1137: return convertToBufferedImage((MultiResolutionImage) img,
> 1138:img.getWidth(null),
> img.getHeight(null));
Any particular reason of using getWidth/getHeight(null) as seems like in spec,
this observer parameter is ignored so we can directly use getWIdth/getHeight, I
presume...
-
PR Review: https://git.openjdk.org/jdk/pull/18187#pullrequestreview-1976081715
PR Review Comment: https://git.openjdk.org/jdk/pull/18187#discussion_r1549294617
PR Review Comment: https://git.openjdk.org/jdk/pull/18187#discussion_r1549296560
Re: RFR: 8226990: GTK & Nimbus LAF: Tabbed pane's background color is not expected one when change the opaque checkbox. [v6]
On Tue, 2 Apr 2024 17:32:26 GMT, Abhishek Kumar wrote:
>> JTabbedPane's content area, tab area and tab background color are not as
>> expected when opaque is set to true or false.
>> The proposed fix is to handle the TabbedPane's background color in installed
>> LAFs. Manual test is added to support the fix and there is no regression
>> caused by the fix.
>>
>> Proposed fix is tested in Ubuntu 22.04 and Oracle linux.
>>
>> CI link is posted in JBS.
>
> Abhishek Kumar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Test update
src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java
line 351:
> 349: Color caretColor = table.getColor("caretColor");
> 350: Color controlText = table.getColor("controlText");
> 351: Color tabbedPaneBg = new Color(238, 238, 238);
Shouldn't it be ColorUIResource?
src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTabbedPaneUI.java
line 776:
> 774:
> 775: // fill content area rect for both GTK and Nimbus LAF here
> 776: g.fillRect(x, y, w, h);
shouldn't it be within if block as is done in BasicTabbedPaneUI so that we fill
only if it's opaque
test/jdk/javax/swing/JTabbedPane/TestJTabbedPaneOpaqueColor.java line 56:
> 54: private static final String INSTRUCTIONS = """
> 55: The background color of panel (which contains the tabbed pane
> 56: is green.
instructions formatting needed..lot of empty spaces in the instruction dialog...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1549254047
PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1549271306
PR Review Comment: https://git.openjdk.org/jdk/pull/17720#discussion_r1549283850
Integrated: 8328541: Remove or update obsolete comment in JRootPane
On Fri, 29 Mar 2024 07:21:17 GMT, Prasanta Sadhukhan wrote: > There is an old obsolete comment in JRootPane that is between the main > documentation comment for the class and the declaration of the class itself > which is causing interference with the work to support Markdown in > documentation comments using the `///` style of comment. (JEP 467) so > removing this comment.. This pull request has now been integrated. Changeset: 3f5b75a5 Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/3f5b75a5ef1606ee9ee0fcefaafcf4a8941788b4 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod 8328541: Remove or update obsolete comment in JRootPane Reviewed-by: aivanov, abhiscxk - PR: https://git.openjdk.org/jdk/pull/18545
Re: RFR: 8328541: Remove or update obsolete comment in JRootPane [v2]
> There is an old obsolete comment in JRootPane that is between the main > documentation comment for the class and the declaration of the class itself > which is causing interference with the work to support Markdown in > documentation comments using the `///` style of comment. (JEP 467) so > removing this comment.. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/18545/files - new: https://git.openjdk.org/jdk/pull/18545/files/c62ba8a7..051cd70e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18545=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18545=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18545.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18545/head:pull/18545 PR: https://git.openjdk.org/jdk/pull/18545
RFR: 8328541: Remove or update obsolete comment in JRootPane
There is an old obsolete comment in JRootPane that is between the main documentation comment for the class and the declaration of the class itself which is causing interference with the work to support Markdown in documentation comments using the `///` style of comment. (JEP 467) so removing this comment.. - Commit messages: - 8328541: Remove or update obsolete comment in JRootPane Changes: https://git.openjdk.org/jdk/pull/18545/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18545=00 Issue: https://bugs.openjdk.org/browse/JDK-8328541 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18545.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18545/head:pull/18545 PR: https://git.openjdk.org/jdk/pull/18545
Re: RFR: 8328561: test java/awt/Robot/ManualInstructions/ManualInstructions.java isn't used
On Thu, 21 Mar 2024 20:00:48 GMT, Phil Race wrote: > I don't know why this file is checked in. It has no @test tag and doesn't do > anything. I am deleting it. ok - Marked as reviewed by psadhukhan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18440#pullrequestreview-1968005195
Re: RFR: 8075917: The regression-swing case failed as the text on label is not painted red with the GTK L [v5]
On Tue, 26 Mar 2024 07:38:39 GMT, Abhishek Kumar wrote: > @aivanov-jdk > > > This looks weird… So you're saying Label[Enabled].textForeground and > > Label[Disabled].textForeground are used for Nimbus (and Synth and GTK) > > instead of Label.foreground and Label.disabledForeground which are used for > > other L > > As per my understanding, Yes, for Nimbus LAF the UI properties are different > than other LAF. > > > Shouldn't we fix the problem by correcting the keys instead? It looks like > > it's what you're doing for specific components. > > I am not sure if it is a problem or nimbus LAF is supposed to be like this. > > > Is it specified anywhere that Synth-based L use different constants? It > > results in incorrect colors. > > Need to check on this. > > > If a developer sets the common properties, should they override > > Look-and-Feel defaults? > > Will check and revert back. As I understood, what `Label.background` color to be set, is determined via **`Nimbus.Overrides`** property i.e., if `Label.background` is set to `Nimbus.Overrides`, it should override the `label.setBackground()` user color setting else if `Nimbus.Overrides` is not set or set to null, then `Label.setBackground()` will hold precedent even if `Label.background` property is set Similarly, if `Nimbus.Overrides` is set with `Label.background` color **AND** Label[Enabled].background is also set, then Label[Enabled].background will have priority over Label.background which will have priority over Label.setBackground() but this is also controlled by another boolean property **`Nimbus.Overrides.InheritDefaults`** in way that the above precedence is valid only if if Nimbus.Overrides.InheritDefaults is true if **`Nimbus.Overrides.InheritDefaults`** is false, then Label[Enabled].background will be ignored and Label.background will have priority over Label.setBackground() This is somewhat explained in `https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/javax/swing/plaf/nimbus/package-summary.html` and tested in` test/jdk/javax/swing/plaf/nimbus/ColorCustomizationTest.java` so in a way, it seems widget[state].color setting is applicable for Nimbus only since we dont have GTK.Overrides property - PR Comment: https://git.openjdk.org/jdk/pull/17763#issuecomment-2019798007
Integrated: 8079786: [macosx] Test java/awt/Frame/DisposeParentGC/DisposeParentGC.java fails for Mac only
On Fri, 22 Mar 2024 09:55:46 GMT, Prasanta Sadhukhan wrote: > Test is now passing in mac..Several iterations of the test passed in CI > system..link in JBS...deproblemlisted... This pull request has now been integrated. Changeset: f67ec19e Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/f67ec19e236e3b4d2d4d4993a7d64b8de052d241 Stats: 34 lines in 2 files changed: 22 ins; 2 del; 10 mod 8079786: [macosx] Test java/awt/Frame/DisposeParentGC/DisposeParentGC.java fails for Mac only Reviewed-by: prr - PR: https://git.openjdk.org/jdk/pull/18449
Re: RFR: 8328730: Convert java/awt/print/bug8023392/bug8023392.html applet test to main [v2]
On Fri, 22 Mar 2024 17:38:36 GMT, Damon Nguyen wrote:
>> Convert java/awt/print/bug8023392/bug8023392.html applet test to main using
>> PassFailJFrame
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move test out of extra dir
test/jdk/java/awt/print/bug8023392.java line 116:
> 114: @Override
> 115: protected void paintComponent(Graphics g) {
> 116: SwingUtilities2.drawChars(this, g, s.toCharArray(),
cant we use `Graphics.drawChars` instead in all places
`SwingUtilities2.drawChars` is used?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18458#discussion_r1536594819
Re: RFR: 8328719: Convert java/awt/print/PageFormat/SetOrient.html applet test to main
On Fri, 22 Mar 2024 18:55:01 GMT, Damon Nguyen wrote:
> Convert java/awt/print/PageFormat/SetOrient.html applet test to main using
> PassFailJFrame.
Marked as reviewed by psadhukhan (Reviewer).
test/jdk/java/awt/print/PageFormat/SetOrient.java line 106:
> 104: pjob.print();
> 105: } catch (PrinterException e) {
> 106: throw new RuntimeException(e.getMessage());
In other PRs, I guess we do
ex.printStackTrace();
String msg = "PrinterException: " + ex.getMessage();
PassFailJFrame.forceFail(msg);
-
PR Review: https://git.openjdk.org/jdk/pull/18462#pullrequestreview-1956340700
PR Review Comment: https://git.openjdk.org/jdk/pull/18462#discussion_r1536593630
Re: RFR: 8328561: test java/awt/Robot/ManualInstructions/ManualInstructions.java isn't used
On Thu, 21 Mar 2024 20:00:48 GMT, Phil Race wrote: > I don't know why this file is checked in. It has no @test tag and doesn't do > anything. I am deleting it. Should we also remove test/jdk/java/awt/Mouse/MouseModifiersUnitTest/ModifierPermutation.java which also does not have @test tag and added for same product fix JDK-6315717? - PR Comment: https://git.openjdk.org/jdk/pull/18440#issuecomment-2016406188
Re: RFR: 8079786: [macosx] Test java/awt/Frame/DisposeParentGC/DisposeParentGC.java fails for Mac only [v2]
> Test is now passing in mac..Several iterations of the test passed in CI > system..link in JBS...deproblemlisted... Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: spacing - Changes: - all: https://git.openjdk.org/jdk/pull/18449/files - new: https://git.openjdk.org/jdk/pull/18449/files/565915f4..429fded4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18449=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18449=00-01 Stats: 13 lines in 1 file changed: 4 ins; 1 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/18449.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18449/head:pull/18449 PR: https://git.openjdk.org/jdk/pull/18449
Integrated: 8328673: Convert closed text/html/CSS manual applet test to main
On Thu, 21 Mar 2024 09:04:19 GMT, Prasanta Sadhukhan wrote: > Couple closed manual applet text/html/CSS tests are converted to main based > and opensourced This pull request has now been integrated. Changeset: cd534f81 Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/cd534f8197341fbe3b3811f5be43c88090e16366 Stats: 161 lines in 2 files changed: 161 ins; 0 del; 0 mod 8328673: Convert closed text/html/CSS manual applet test to main Reviewed-by: abhiscxk, aivanov - PR: https://git.openjdk.org/jdk/pull/18424
Re: RFR: 8328484: Convert and Opensource few JFileChooser applet test to main
On Fri, 22 Mar 2024 08:55:26 GMT, Abhishek Kumar wrote: > Few closed manual applet JFileChooser tests convereted to PassFailJFrame > based manual test. Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18447#pullrequestreview-1954414137
RFR: 8079786: [macosx] Test java/awt/Frame/DisposeParentGC/DisposeParentGC.java fails for Mac only
Test is now passing in mac..Several iterations of the test passed in CI system..link in JBS...deproblemlisted... - Commit messages: - 8079786: [macosx] Test java/awt/Frame/DisposeParentGC/DisposeParentGC.java fails for Mac only Changes: https://git.openjdk.org/jdk/pull/18449/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18449=00 Issue: https://bugs.openjdk.org/browse/JDK-8079786 Stats: 21 lines in 2 files changed: 18 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18449.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18449/head:pull/18449 PR: https://git.openjdk.org/jdk/pull/18449
Re: RFR: 8316324: Opensource five miscellaneous Swing tests
On Thu, 21 Mar 2024 23:35:58 GMT, Alexander Zuev wrote:
> Clean up and opensource five tests.
test/jdk/javax/swing/InputVerifier/bug4774166.java line 44:
> 42: * @summary InputVerifier should be called after a window loses and then
> regains focus
> 43: * @library /javax/swing/regtesthelpers
> 44: * @build JRobot
I guess JRobot library dependancy is not needed unnecessarliy, could be
achieved by normal Robot in this test..
test/jdk/javax/swing/JButton/4385611/bug4385611.java line 42:
> 40: public class bug4385611 {
> 41: static JButton bt1, bt2;
> 42: static final ImageIcon icon32x32 = new
> ImageIcon(bug4385611.class.getResource("red.gif"));
did you verify if we are allowed to keep this red.gif in open source as per the
image license? Can we not create the Imageicon from a yellow32x32 bufferedimage?
test/jdk/javax/swing/plaf/motif/bug4150591.java line 40:
> 38: public class bug4150591 {
> 39: public static void main(String[] args) {
> 40: MotifInternalFrameTitlePane mtp = new
> MotifInternalFrameTitlePane(new JInternalFrame());
Is this test required to be added now with motif being deprecated?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18443#discussion_r1535128580
PR Review Comment: https://git.openjdk.org/jdk/pull/18443#discussion_r1535132200
PR Review Comment: https://git.openjdk.org/jdk/pull/18443#discussion_r1535140010
Re: RFR: 8328382: Convert java/awt/FileDialog/FileDialogForPackages test to main [v3]
On Fri, 22 Mar 2024 05:15:38 GMT, Alexander Zuev wrote: >> Convert test to a main based; >> Move test to an appropriate folder; >> Remove old files; > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Got rid of the test window, using OassFailJFrame instead; > Made all variables local; > Minor changes based on review comments; Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18360#pullrequestreview-1954019349
Re: RFR: 8328370: Convert java/awt/print/Dialog/PrintApplet.java applet test to main [v4]
On Thu, 21 Mar 2024 16:11:44 GMT, Damon Nguyen wrote: >> Convert java/awt/print/Dialog/PrintApplet.java applet test to main using >> PassFailJFrame. Also rename the test to PrintModalDialog.java since this is >> no longer an applet. Added test instructions related to the linked JBS issue. > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Fix main/manual Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18362#pullrequestreview-1952852373
Re: RFR: 8328670: Automate and open source few closed manual applet test
On Thu, 21 Mar 2024 10:46:41 GMT, Abhishek Kumar wrote:
> Few manual closed applet test converted to automated and open sourced.
Marked as reviewed by psadhukhan (Reviewer).
test/jdk/javax/swing/JInternalFrame/Ctrli.java line 126:
> 124: keyecho.addMouseListener(mouseListener);
> 125: frame.setLayout(new BorderLayout());
> 126: frame.getContentPane().add(keyecho);
getContentPane is obsolete since jdk1.5 so can be removed as per this
[comment](https://github.com/openjdk/jdk/pull/18390#discussion_r1533017080)
test/jdk/javax/swing/JMenuItem/JActionCommandTest.java line 135:
> 133: mb.add(m);
> 134: f.setJMenuBar(mb);
> 135: f.getContentPane().add("South", tf);
same as above..
-
PR Review: https://git.openjdk.org/jdk/pull/18428#pullrequestreview-1952802096
PR Review Comment: https://git.openjdk.org/jdk/pull/18428#discussion_r1534273918
PR Review Comment: https://git.openjdk.org/jdk/pull/18428#discussion_r1534274838
Re: RFR: 8328382: Convert java/awt/FileDialog/FileDialogForPackages test to main [v2]
On Tue, 19 Mar 2024 14:10:52 GMT, Alexander Zuev wrote: >> Convert test to a main based; >> Move test to an appropriate folder; >> Remove old files; > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Adding library and build clauses test/jdk/java/awt/FileDialog/FileDialogForPackages.java line 59: > 57: frame.add(textScrollPane, BorderLayout.CENTER); > 58: > 59: fd = new FileDialog(new Frame(), "Open"); is there any need to mix Swing and AWT components in the test? I guess you can use the JFrame already created test/jdk/java/awt/FileDialog/FileDialogForPackages.java line 60: > 58: > 59: fd = new FileDialog(new Frame(), "Open"); > 60: fd.setDirectory(APPLICATIONS_FOLDER); guess APPLICATION_FOLDER can be made local test/jdk/java/awt/FileDialog/FileDialogForPackages.java line 91: > 89: .rows((int) instructions.lines().count() + 1) > 90: .columns(40) > 91: .testUI(FileDialogForPackages::initialize) It seems the filedialog is obscuring the instruction and test window...I guess it will be good if you can use position(PassFailJFrame.Position.TOP_LEFT_CORNER) - PR Review Comment: https://git.openjdk.org/jdk/pull/18360#discussion_r1534230960 PR Review Comment: https://git.openjdk.org/jdk/pull/18360#discussion_r1534230082 PR Review Comment: https://git.openjdk.org/jdk/pull/18360#discussion_r1534232128
Integrated: 8328570: Convert closed JViewport manual applet tests to main
On Thu, 21 Mar 2024 03:44:01 GMT, Prasanta Sadhukhan wrote: > Few closed manual applet JViewport tests are converted to main based and > opensourced This pull request has now been integrated. Changeset: 725d87bb Author:Prasanta Sadhukhan URL: https://git.openjdk.org/jdk/commit/725d87bbc2abae2ca856d91668f0a494f93fca1c Stats: 428 lines in 5 files changed: 428 ins; 0 del; 0 mod 8328570: Convert closed JViewport manual applet tests to main Reviewed-by: abhiscxk, kizune - PR: https://git.openjdk.org/jdk/pull/18418
Re: RFR: 8328570: Convert closed JViewport manual applet tests to main [v3]
On Thu, 21 Mar 2024 14:40:30 GMT, Alexander Zuev wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> formatting
>
> test/jdk/javax/swing/JViewport/bug4137282.java line 86:
>
>> 84: }
>> 85:
>> 86: static void setPaneSize(int w, int h) {
>
> Do we even need this method? It is used twice and if we just do setSize and
> setPreferredSize in these two places we could drop this method and make panel
> variable local. That will make it more compact and in my opinion more
> readable.
Yes, I considered that but it has 2 lines and used in 2 places...if it had been
1 line I would have done it but since it was more than 1 line I kept it and I
dont think its affecting readability so much, is it?
> test/jdk/javax/swing/JViewport/bug4243479.java line 30:
>
>> 28: * @library /java/awt/regtesthelpers
>> 29: * @build PassFailJFrame
>> 30:@run main/manual bug4243479
>
> Suggestion:
>
> * @run main/manual bug4243479
ok
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18418#discussion_r1534057018
PR Review Comment: https://git.openjdk.org/jdk/pull/18418#discussion_r1534053111
Re: RFR: 8328570: Convert closed JViewport manual applet tests to main [v4]
> Few closed manual applet JViewport tests are converted to main based and > opensourced Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: formatting - Changes: - all: https://git.openjdk.org/jdk/pull/18418/files - new: https://git.openjdk.org/jdk/pull/18418/files/8e177faa..d588adcc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18418=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18418=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18418.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18418/head:pull/18418 PR: https://git.openjdk.org/jdk/pull/18418
Re: RFR: 8328673: Convert closed text/html/CSS manual applet test to main [v2]
On Thu, 21 Mar 2024 14:00:17 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> library path fix
>
> test/jdk/javax/swing/text/html/CSS/bug4271058.java line 60:
>
>> 58: .columns(30)
>> 59: .testUI(bug4271058::createTestUI)
>> 60: .build()
>
> I guess the test will benefit from a screenshot feature to capture how the
> table is rendered.
> Suggestion:
>
> .testUI(bug4271058::createTestUI)
> .screenCapture()
> .build()
>
>
> The feature to take a screenshot automatically, when Fail button is pressed,
> hasn't been implemented yet. It's tracked under
> [JDK-8317114](https://bugs.openjdk.org/browse/JDK-8317114).
ok
> test/jdk/javax/swing/text/html/CSS/bug4286458.java line 57:
>
>> 55:
>> 56: String text =
>> 57: "" +
>
> Suggestion:
>
> "" +
>
> It should start with an opening tag.
yes
> test/jdk/javax/swing/text/html/CSS/bug4286458.java line 65:
>
>> 63: jep.setEditorKit(new HTMLEditorKit());
>> 64: jep.setEditable(false);
>> 65: jep.setText(text);
>
> Suggestion:
>
> JEditorPane jep = new JEditorPane("text/html", text);
> jep.setEditable(false);
>
> Could be reduced to.
ok
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18424#discussion_r1534048754
PR Review Comment: https://git.openjdk.org/jdk/pull/18424#discussion_r1534049280
PR Review Comment: https://git.openjdk.org/jdk/pull/18424#discussion_r1534048927
Re: RFR: 8328673: Convert closed text/html/CSS manual applet test to main [v3]
> Couple closed manual applet text/html/CSS tests are converted to main based > and opensourced Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Review comment update - Changes: - all: https://git.openjdk.org/jdk/pull/18424/files - new: https://git.openjdk.org/jdk/pull/18424/files/d846e61f..fbc0b821 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18424=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18424=01-02 Stats: 6 lines in 2 files changed: 1 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18424.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18424/head:pull/18424 PR: https://git.openjdk.org/jdk/pull/18424
Re: RFR: 8328570: Convert closed JViewport manual applet tests to main [v3]
> Few closed manual applet JViewport tests are converted to main based and > opensourced Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: formatting - Changes: - all: https://git.openjdk.org/jdk/pull/18418/files - new: https://git.openjdk.org/jdk/pull/18418/files/395730f3..8e177faa Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18418=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18418=01-02 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18418.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18418/head:pull/18418 PR: https://git.openjdk.org/jdk/pull/18418
