RFR: 8257810: Only First page are printed in JTable.scrollRectToVisible [v6]

[Alexey Ivanov]

On Fri, 8 Apr 2022 12:41:29 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Issue was when printing a JTable which sits inside a JScrollPane and the table is scrolled down to the end to about 1000th row, only the first page is printed. 
>> This is because when the table is scrolled down to last page, the bounds.y becomes -ve 
>> [x=0,y=-15260,width=968,height=16000]
>> so the check `if (!((table.getBounds()).intersects(clip)))` is satisfied only for 1st page where bounds just intersects the clip [x=0,y=0,width=968,height=1296]
>> but subsequent pages clip 
>> [[x=0,y=1296,width=968,height=1296], 
>> [x=0,y=2592,width=968,height=1296], 
>> [x=0,y=3888,width=968,height=1296] etc is not intesecting so they are not printed
>> 
>> This is a regression of JDK-8081491 which was **reworked** in JDK-8236907 where the bounds calculation and usage is made same as in BasicTableUI
>> We need to use the same resetted bounds for this intersection calculation too as was done for JDK-8236907
>> 
>> Tested against JDK-8081491, 8170349, JDK-8236907 testcases along with other regression tests and all are OK (link in JBS)
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Test fix

Changes requested by aivanov (Reviewer).

> I will rather not make the test more complicated...it's not a test fix..it just to verify the product fix..
> I am not able to close in windows using ALT+F4 anyway...
> Other suggestions accepted...

It doesn't make the code *too* complicated. And it makes it complete so that it can be used as the start for other tests.

Would you be able to apply the following patch?


diff --git a/test/jdk/javax/swing/JTable/PrintAllPagesTest.java b/test/jdk/javax/swing/JTable/PrintAllPagesTest.java
index ec0066088bd..8c61639d03d 100644
--- a/test/jdk/javax/swing/JTable/PrintAllPagesTest.java
+++ b/test/jdk/javax/swing/JTable/PrintAllPagesTest.java
@@ -28,6 +28,8 @@
  */
 import java.awt.BorderLayout;
 import java.awt.FlowLayout;
+import java.awt.event.WindowAdapter;
+import java.awt.event.WindowEvent;
 import java.awt.print.PrinterException;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -43,35 +45,32 @@ import javax.swing.table.AbstractTableModel;
 import javax.swing.table.TableModel;
 import javax.swing.WindowConstants;
 
-public class PrintAllPagesTest {
+public class PrintAllPagesTest extends WindowAdapter {
     static JFrame f;
     static JDialog dialog;
     static JTable table;
     static volatile boolean testResult = false;
-    final static CountDownLatch latch = new CountDownLatch(1);
-    static  boolean ret = false;
+    static final CountDownLatch latch = new CountDownLatch(1);
 
     public static void main(String[] args) throws Exception {
 
         try {
             SwingUtilities.invokeAndWait(() -> {
-                createUI();
                 printAllPagesTest();
+                createUI();
             });
 
             Thread.sleep(1000);
             SwingUtilities.invokeAndWait(() -> {
                 try {
-                    ret = table.print();
+                    if (!table.print()) {
+                        throw new RuntimeException("Printing cancelled");
+                    }
                 } catch (PrinterException e) {
                     throw new RuntimeException("Printing failed: " + e);
                 }
             });
 
-            if (!testResult) {
-                throw new RuntimeException("Only 1st page is printed out of multiple pages");
-            }
-
             // wait for latch to complete
             if (!latch.await(5, TimeUnit.MINUTES)) {
                 throw new RuntimeException("Test timed out");
@@ -120,10 +119,10 @@ public class PrintAllPagesTest {
         f = new JFrame("Table test");
         f.add(scrollpane);
         f.setSize(1000, 800);
-        f.setUndecorated(true);
         f.setLocationRelativeTo(null);
         f.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
         f.setVisible(true);
+        f.addWindowListener(new PrintAllPagesTest());
     }
 
     private static void createUI() {
@@ -133,8 +132,7 @@ public class PrintAllPagesTest {
                 + " If only 1 page is printed,\n "
                 + " then press fail else press pass";
 
-        dialog = new JDialog();
-        dialog.setTitle("textselectionTest");
+        dialog = new JDialog(f, "Instructions for Table Print Test");
         JTextArea textArea = new JTextArea(description);
         textArea.setEditable(false);
         final JButton passButton = new JButton("PASS");
@@ -154,8 +152,13 @@ public class PrintAllPagesTest {
         buttonPanel.add(failButton);
         mainPanel.add(buttonPanel, BorderLayout.SOUTH);
         dialog.add(mainPanel);
-        dialog.setUndecorated(true);
         dialog.pack();
         dialog.setVisible(true);
+        dialog.addWindowListener(new PrintAllPagesTest());
+    }
+
+    @Override
+    public void windowClosing(WindowEvent e) {
+        latch.countDown();
     }
 }


The reason why I changed the order and call `printAllPagesTest()` before `createUI()` is to make sure `f` is not `null` when the dialog is created. The owned dialog is always displayed above its owner, this addresses two issues together: it will be brought up whenever the frame is activated; if the screen resolution is low, the dialog could become hidden below the frame leaving the tester with no instructions visible.

I used the blessed order of modifiers: `static final`.

Closing either the dialog or the frame fails the test. Yet I didn't bother to provide a more detailed message; the test fails because `testResult` remains `false`.

test/jdk/javax/swing/JTable/PrintAllPagesTest.java line 73:

> 71:             if (!testResult) {
> 72:                 throw new RuntimeException("Only 1st page is printed out of multiple pages");
> 73:             }

Now the test always fails whether printing is called or not.

I handled this situation in the patch I proposed.

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

PR: https://git.openjdk.java.net/jdk/pull/8141