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

[Sergey Bylokhov]

Looks fine.

On 12.11.15 14:20, Alexander Scherbatiy wrote:
>
>    The fix looks good to me.
>
>    Thanks,
>    Alexandr.
>
> On 11/11/2015 7:19 PM, Pete Brunet wrote:
>> 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
>>>>>>>>>
>


-- 
Best regards, Sergey.