<Swing Dev> [14] RFR 8226513:, JEditorPane is shown with incorrect size

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Mon Aug 12 10:11:20 UTC 2019


Hi Sergey,

When frame.setVisible() is called we do

Frame.setVisible() -> BorderLayout.layoutContainer() where we do
if ((c=getChild(CENTER,ltr)) != null) {
             c.setBounds(left, top, right - left, bottom - top);
         }

Now, in linux we get
bottom = target.height - insets.bottom; 0-5=-5
and right = target.width - insets.right; 0-5=-5

right=-5, left=5, bottom=-5,top=25

so component's or JRootPane's bounds width=-10,height=-30

Now, when we call frame.pack(), we do GridLayout.preferredLayoutSize() 
where Component comp = parent.getComponent(i) returns JTextField as 
component with -10x-15 as width,height

Regards
Prasanta
On 12-Aug-19 2:25 PM, Sergey Bylokhov wrote:
> Hi, Prasanta.
>
> On 8/12/19 1:41 am, Prasanta Sadhukhan wrote:
>> as I saw that Dimension d returns negative width(-10) and height 
>> (-15) in JEditorPane.getPreferredSize() when bug8001470.java is run 
>> on linux.
>
> But why this size is negative?
>
>>
>> The "rootViewNeedsLayout" flag causes this check to be not used and 
>> still pass and I think this is needed.
>>
>>
>> Also, as told, the hidpi issue of the present test is caused by 8178025
>> <https://bugs.openjdk.java.net/browse/JDK-8178025>and should need 
>> some SwingUtilities2.isScaleChanged check somewhere, which I intend 
>> to work on in 8229222 
>> <https://bugs.openjdk.java.net/browse/JDK-8229222>, if you do not 
>> have any objection.
>>
>>
>> Regards
>>
>> Prasanta
>>
>> On 10-Aug-19 2:12 AM, Sergey Bylokhov wrote:
>>> Hi, Prasanta.
>>>
>>> Since this is the second regression of JDK-8130892, my suggestion 
>>> was to try to revert all of these fixes and check that such changes 
>>> are really necessary. Do we really need to change the check from
>>> "d.width <= 0 || d.height <= 0" to "d.width <= 0 || d.height <= 0", 
>>> and do wee need a flag?
>>>
>>> I do not suggest to re-implement the fix, but recheck that we will 
>>> not integrate on more regression.
>>>
>>> ----- prasanta.sadhukhan at oracle.com wrote:
>>> >
>>>
>>> Hi Sergey,
>>>
>>>
>>> >
>>>
>>> It seems the hidpi issue of the test is because of JDK-8178025 
>>> <https://bugs.openjdk.java.net/browse/JDK-8178025> where we 
>>> calculate the fontmetrics because of scaleChange
>>> >
>>>
>>> /jdk10-b33/bin/java JEditorPaneLayoutTest
>>> > Exception in thread "main" java.lang.RuntimeException: Wrong size 
>>> java.awt.Dimension[width=180,height=10] expected 
>>> java.awt.Dimension[width=180,height=44]
>>> >         at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:87)
>>> >
>>> >
>>>
>>> jdk10-b34/bin/java JEditorPaneLayoutTest
>>> > Exception in thread "main" java.lang.RuntimeException: Wrong size 
>>> java.awt.Dimension[width=181,height=10] expected 
>>> java.awt.Dimension[width=180,height=44]
>>> >         at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:87)
>>>
>>>
>>> >
>>>
>>> Since all the regression tests for this issue like
>>> >
>>>
>>> javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java, 
>>> javax/swing/JTextArea/ScrollbarFlicker/ScrollFlickerTest.java and 
>>> the present test
>>>
>>> is passing with the fix and I have already created a JBS 
>>> issueJDK-8229222 
>>> <https://bugs.openjdk.java.net/browse/JDK-8229222>to work on the 
>>> test issue, which is not directly related to this fix, can we check 
>>> this in and I work on the JBS issue subsequently?
>>>
>>>
>>> >
>>>
>>> Regards
>>>
>>> Prasanta
>>> >
>>>
>>> On 09-Aug-19 2:43 AM, Sergey Bylokhov wrote:
>>> >
>>>
>>>     >
>>>     Hi, Prasanta.
>>>
>>>     >
>>>     I have one additional suggestion. I think we need to check the 
>>> whole related sequence of bugs
>>>     >
>>>
>>>     >
>>>     0) JDK-8001470 - The bug which is unrela6ed to the current bug, 
>>> but it is introduce the test from which we start the step (1)
>>>
>>>     >
>>>     1) Initial bug was reproduced on Solaris 11 only: 
>>> https://bugs.openjdk.java.net/browse/JDK-8130892
>>>      As a fix the check for "uninitialized" state was changed, but 
>>> it was caused next regression
>>>     >
>>>
>>>     >
>>>     2) https://bugs.openjdk.java.net/browse/JDK-8160246 , it also 
>>> reproduced only on Solaris and this is the fix where we added the 
>>> "rootViewInitialised" flag, but it cause next regression
>>>
>>>     >
>>>     2) Current bug https://bugs.openjdk.java.net/browse/JDK-8226513
>>>     >
>>>
>>>     >
>>>     And you already found by the test that it dose not take into 
>>> account the fact that the GConfig may be different when you 
>>> calculate the size for invisible component, and the size after it is 
>>> became visible, probably ithe fix itself does not take into account 
>>> this?
>>>
>>>     >
>>>     It is also quite interesting to check the root cause of the 
>>> JDK-8130892 :
>>> https://bugs.openjdk.java.net/browse/JDK-8130892?focusedCommentId=13822701&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13822701
>>>
>>>     >
>>>     I doubt that negative size is a correct value for the component.
>>>     >
>>>
>>>     >
>>>     ----- philip.race at oracle.com wrote:
>>>     > >
>>>     OK +1 from me !
>>>     > >
>>>     > > -phil.
>>>     > >
>>>     > >
>>>     > > On 8/8/19 2:23 AM, Prasanta Sadhukhan wrote:
>>>     > >
>>>
>>>
>>>         > >
>>>
>>>         > > On 07-Aug-19 9:42 PM, Phil Race wrote:
>>>         > >
>>>
>>>             I have a minor quibble that "rootViewInitialized" is no 
>>> longer a very appropriate variable name,
>>>             > > since it is not just about initialization.
>>>             > > Can we rename it to "rootViewNeedsLayout" ?
>>>             > >
>>>             > >
>>>         Renamed.
>>>         > >
>>>
>>>
>>>             > > Also because the windows 10 issue has a different 
>>> cause, potentially this test can be adjusted
>>>             > > to allow some tolerance to tell the difference between
>>>             > > "not re-layed out at all", and "I'm a few pixels off 
>>> in my expectations".
>>>             > > ie the test should be about "did I relayout?", not 
>>> "is my preferred size exactly the actual size?".
>>>             > >
>>>             > > Could this be a hi-dpi issue ? Are you running at 96 
>>> dpi or something else when this fails ?
>>>             > >
>>>             > >
>>>         Yes, it is. I have modified the jtreg command line to test 
>>> in uiScale 1. It now pass in windows and others and fails without 
>>> the fix.
>>>
>>> http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.02/
>>>
>>>         Regards
>>>         > > Prasanta
>>>         > >
>>>
>>>
>>>             > > -phil.
>>>             > >
>>>             > >
>>>             > >
>>>             > > On 8/7/19 2:04 AM, Prasanta Sadhukhan wrote:
>>>             > >
>>>
>>>                 I confirm that the test pass on linux and mac with 
>>> the fix and fail without the fix. Only on windows, it fails.
>>>                 > >
>>>
>>>                 But I also see that the failure is not because of 
>>> the fix. The "problem" was there even before the fix, for example 
>>> with jdk13 (fails with jdk11, jdk12 too )
>>>                 > >
>>>
>>>                 jdk13-b24/bin/java JEditorPaneLayoutTest
>>>                 > > Exception in thread "main" 
>>> java.lang.RuntimeException: Wrong size 
>>> java.awt.Dimension[width=183,height=10] expected 
>>> java.awt.Dimension[width=177,height=44]
>>>                 > >         at 
>>> JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
>>>                 > >
>>>
>>>                 which got somewhat improved by JDK-8217731 
>>> <https://bugs.openjdk.java.net/browse/JDK-8217731>
>>>
>>>                 jdk13-b25/bin/java JEditorPaneLayoutTest
>>>                 > > Exception in thread "main" 
>>> java.lang.RuntimeException: Wrong size 
>>> java.awt.Dimension[width=181,height=10] expected 
>>> java.awt.Dimension[width=180,height=44]
>>>                 > >         at 
>>> JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
>>>
>>>                 So, the 1 pixel difference in width was there even 
>>> before this fix and also this fix fixes the height of the basic text 
>>> component and the unit tests of JBS works ok, so to me this fix 
>>> looks ok to me.
>>>
>>>                 +1 for the fix from me and
>>>                 > >
>>>
>>>                 I have added the test to problem list only for 
>>> windows to figure out the 1 pixel difference in width which seems to 
>>> be because of layouting in windows.
>>>                 > >
>>>
>>> http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.01/
>>>
>>>                 Regards
>>>                 > > Prasanta
>>>                 > >
>>>                 > > On 22-Jul-19 4:01 PM, Prasanta Sadhukhan wrote:
>>>                 > >
>>>
>>>                     Hi Semyon,
>>>                     > >
>>>                     > > Although the JBS testcase is passing with 
>>> your change, your testcase is failing even with the fix for me in 
>>> windows10
>>>                     > >
>>>                     > > java.lang.RuntimeException: Wrong size 
>>> java.awt.Dimension[width=181,height=44] expected 
>>> java.awt.Dimension[width=180,height=44]
>>>                     > >         at 
>>> JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84)
>>>                     > >
>>>                     > > Regards
>>>                     > >
>>>                     > > Prasanta
>>>                     > >
>>>                     > > On 17-Jul-19 1:33 AM, 
>>> semyon.sadetsky at oracle.com wrote:
>>>                     > >
>>>
>>>                         bug: 
>>> https://bugs.openjdk.java.net/browse/JDK-8226513
>>>                         > >
>>>                         > > webrev: 
>>> http://cr.openjdk.java.net/~ssadetsky/8226513/webrev.00/
>>>                         > >
>>>                         > > The fix adds resetting the root view 
>>> initialization flag when the text component underling document is 
>>> changed and also removes the check for the zero component size for 
>>> the root view initialization to correct the resulting preferred 
>>> component size in some scenarios when the root view need to be 
>>> initially layouted.
>>>                         > >
>>>                         > > --Semyon
>>>                         > >
>>>                         > >
>>>                         > >
>>>
>>>             > >
>>>
>>>     > >
>>>
>
>


More information about the swing-dev mailing list