<Swing Dev> RfR JDK-8134116, Add more comprehensive fix and regression test for JDK-8133897

Pete Brunet peter.brunet at oracle.com
Wed Nov 11 16:19:12 UTC 2015


Thanks Alexandr, I made that change and added a test to ensure
getAccessibleName returns the proper page when the page's component is
null.  Please see the following webrev.

http://cr.openjdk.java.net/~ptbrunet/JDK-8134116/webrev.05/

Pete

On 11/11/15 2:25 AM, Alexander Scherbatiy wrote:
> 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