<Swing Dev> RfR JDK-8134116, Add more comprehensive fix and regression test for JDK-8133897
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Nov 11 08:25:16 UTC 2015
On 11/10/2015 11:52 PM, Pete Brunet wrote:
> Thanks Sergey, Please see
>
> http://cr.openjdk.java.net/~ptbrunet/JDK-8134116/webrev.04/
>
> parent.indexOfTab(title) is replaced with
> parent.indexOfTabComponent(tabComponent).
>
> The regression test runs OK.
It looks like the check to null component in the getTitleMethod() is
not necessary.
Even if a page contains a null component the
parent.indexOfComponent(null) returns the correct index.
So we always asks for a component (it may be null) which definitely
is containded in the pages list.
Here is a small example:
---------------
tabbedPane.addTab("Label 1", new JLabel("Label 1"));
tabbedPane.addTab("Null", null);
tabbedPane.addTab("Label 2", new JLabel("Label 2"));
System.out.println("index of null component: " +
tabbedPane.indexOfComponent(null)); // returns index 1
---------------
Thanks,
Alexandr.
>
> Pete
>
> On 11/10/15 6:12 AM, Sergey Bylokhov wrote:
>> Hi, Pete.
>> On 31.10.15 6:28, Pete Brunet wrote:
>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8134116/webrev.03/
>> My suggestion was to remove all usage of parent.indexOfTab(title) in
>> the code and replace it by parent.indexOfTabComponent(comp).
>> For example:
>> getTabBounds(parent, parent.indexOfTab(getTitle()));
>> Can return incorrect bounds if a few pages will have the same title.
>>
>> Another problem in the fix is that it iterates over components twice:
>> in the getTitle()->(parent.indexOfComponent(component)), and in the
>> parent.indexOfTab(title).
>>
>> Please do not use such comments in the code "// For
>> JDK-8133897/JDK-8134116", this information can be obtained from the
>> mercurial history.
>>
>>
>>
>>>>> I guess it will be better to don't use the title (especially
>>>>> parent.indexOfTab(title)) at all in the our code, except the situation
>>>>> when we should access the title(like in getAccessibleName()). All
>>>>> other cases should be rewritten to use
>>>>> parent.indexOfTabComponent(comp). For example your "private String
>>>>> getTitle() {}" can be implemented like this:
>>>>>
>>>>> return getTitleAt(parent.indexOfTabComponent(comp));
>>>>>
>>>>> On 20.10.15 18:45, Pete Brunet wrote:
>>>>>> Please review this patch:
>>>>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8134116/webrev.02/
>>>>>>
>>>>>> The issue raised/fixed in 8133897 and now resolved in a better
>>>>>> fashion
>>>>>> in this patch is caused by an override of the functionality of
>>>>>> JTabbedPane such that its Page inner class title field is not kept
>>>>>> up to
>>>>>> date by the overriding code. When the Page title field is empty
>>>>>> getTitleAt is now called so that the overridden getTitleAt will
>>>>>> provide
>>>>>> the title.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134116
>>>>>>
>>>>>> Pete
>>>>>>
>>
More information about the swing-dev
mailing list