<Swing Dev> Review request for 4473075: JTable header rendering problem (after setting preferred size)

[Sergey Bylokhov]

Hi, Semyon.
  A few notes:
  - Please rephrase "Returns a preferred size of the TableHeader". Use 
the class name JTableHeader or something like this: "Returns the table 
header's...."
  - The frame should be disposed at the end of the test.

Small issues, not necessary to be fixed:
  - You can add @override to the new method
  - The test can iterate over all supported l&f.

On 03.03.2015 16:53, Semyon Sadetsky wrote:
> synchronization is not necessary because the invokeAndWait() method 
> guarantee that the variable will never be accessed from EDT after it.
>
> On 3/3/2015 4:45 PM, Alexander Scherbatiy wrote:
>> On 3/3/2015 3:30 PM, Semyon Sadetsky wrote:
>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/4473075/webrev.02/
>>>
>>> null check is not needed because isPreferredSizeSet() is true only 
>>> if preferred size is not null.
>>> Others are accepted.
>>    The point field is used both on the main and EDT thread in the 
>> test. There should be proper synchronization to the fields access.
>>
>>    Otherwise  the fix looks good to me.
>>
>>    Thanks,
>>    Alexandr.
>>
>>>
>>>
>>> On 3/3/2015 1:09 PM, Alexander Scherbatiy wrote:
>>>>
>>>>   The fix looks good to me.
>>>>
>>>>   Just few comments:
>>>>   - Check that getPreferredSize() can't return null if prefferd 
>>>> size is set
>>>>   - extra empty lines 442 and 463 are unnecessary
>>>>   - scpScroll.getHorizontalScrollBar() call (line 69) in the test 
>>>> is called on the main thread. It should be called on EDT.
>>>>
>>>>  Thanks,
>>>>  Alexandr.
>>>>
>>>> On 3/2/2015 4:11 PM, Semyon Sadetsky wrote:
>>>>> Regression test scenario was added. Please review the fix.
>>>>>
>>>>> webrev: 
>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/4473075/webrev.01/ 
>>>>>
>>>>>
>>>>> 2Sergey:
>>>>> the garbage mentioned comments in the JBS is not reproducible 
>>>>> anymore. In any case it is another problem than the described in 
>>>>> the bug header. The fix solves the original issue.
>>>>>
>>>>>
>>>>> On 2/27/2015 4:47 PM, Semyon Sadetsky wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> It really depends on a point of view.
>>>>>> There is a comment in the ViewportLayout.java that states that it 
>>>>>> is an expected behaviour:
>>>>>> /* If the new viewport size would leave empty space to the
>>>>>>   * right of the view, right justify the view or left justify
>>>>>>   * the view when the width of the view is smaller than the
>>>>>>   * container.
>>>>>>   */
>>>>>> ==
>>>>>> Semyon
>>>>>>
>>>>>> On 2/27/2015 4:06 PM, Sergey Bylokhov wrote:
>>>>>>> Hi, Semyon.
>>>>>>> Can you clarify the comment below from the CR. Is the actual bug 
>>>>>>> in JTableHeader and not in JScrollPane? Especially consider, 
>>>>>>> that the behavior changes by different modes of a JScrollPane.
>>>>>>>
>>>>>>> "This problem isn't unique to JTableHeader. For example, 
>>>>>>> consider the test case I've attached, Bug2.java. Run it the same 
>>>>>>> way, resized so that the label header component shows "..." at 
>>>>>>> the end, and then scroll.
>>>>>>>
>>>>>>> This appears to be a problem with how JScrollPane treats any 
>>>>>>> header component with a fixed size like this. There are two 
>>>>>>> things here:
>>>>>>>
>>>>>>> First, I don't understand why garbage is allowed to show up. We 
>>>>>>> at least need to fix this. Second, perhaps JScrollPane needs to 
>>>>>>> do better to ensure that the header is large enough to match the 
>>>>>>> scrollable content."
>>>>>>>
>>>>>>>
>>>>>>> Also I think it is possible to write a test for this issue.
>>>>>>>
>>>>>>> -----semyon.sadetsky at oracle.com  wrote:
>>>>>>>
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> please review a fix for JDK 9.
>>>>>>>>
>>>>>>>> Bug:https://bugs.openjdk.java.net/browse/JDK-4473075
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/4473075/webrev.00/ 
>>>>>>>>
>>>>>>>>
>>>>>>>> The thing is the general logic of the viewport layout requires the
>>>>>>>> right
>>>>>>>> component size which is requested by the getPreferredSize call 
>>>>>>>> to the
>>>>>>>>
>>>>>>>> child component.
>>>>>>>> The right width of JTableHeader should be calculated in its UI 
>>>>>>>> but if
>>>>>>>>
>>>>>>>> user sets the preferred size manually it makes the calculated size
>>>>>>>> invisible outside the component. User may need to set preferred 
>>>>>>>> size
>>>>>>>> in
>>>>>>>> order to adjust header height but in 
>>>>>>>> setPreferredSize(Dimension) the
>>>>>>>> header width is adjusted as well.
>>>>>>>> The fix solution overrides JComponent::getPreferredSize in
>>>>>>>> JTableHeader
>>>>>>>> with the logic returning the right width while preserving the 
>>>>>>>> height
>>>>>>>> adjustable for user.
>>>>>>
>>>>>
>>>>
>>>
>>
>


-- 
Best regards, Sergey.