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 >>>>>>> >>> >
