<Swing Dev> [9] Review request for 8036983 - JAB:Multiselection Ctrl+CursorUp/Down and ActivateDescenderPropertyChanged event

[Vivi An]

Hello Alex,

On 4/11/2014 6:21 AM, Alexander Scherbatiy wrote:
>
> src/share/classes/javax/swing/tree/DefaultTreeSelectionModel.java
> 208      * The lead path is set to the last unique path.
>
> This comment contradicts with the:
> http://docs.oracle.com/javase/7/docs/api/javax/swing/tree/TreeSelectionModel.html 
>
>    The lead TreePath is the last path that was added (or set).
> and with:
> http://docs.oracle.com/javase/7/docs/api/javax/swing/tree/DefaultTreeSelectionModel.html#leadPath 
>
>   protected TreePath leadPath
>   Last path that was added.
Hmm, the preceding comment of 208 was the old one,  did not feel like 
conflict with the doc. I added "selected" to try to make it clearer. 
How's change to: "The lead path is last tree path that was added or 
set." to fully adapt to the doc.
>
> Could you also run the JTable and JTree JCK tests?
64 Tests all passed on Windows, report attached to the bug
>
> Thanks,
> Alexandr.
>
> On 4/11/2014 3:24 AM, Vivi An wrote:
>> Thanks Petr
>>
>> Please see comments inline. Plus, one more question, does new package 
>> private function requires CCC approval?
>>
>> ~ Vivi
>>
>> On 4/10/2014 7:25 AM, Petr Pchelko wrote:
>>> Hello, Vivi.
>>>
>>>>> Is it possible to write a test (manual or automated) for the fix?
>>>> Two test files (One for JTree, one for JTable) attached to the bug 
>>>> for manual test,  comments added in each file about the how to do 
>>>> the test.
>>> Is is possible to make an automatic test?
>>> You could show the JTree, manually get it’s AccessibleContext and 
>>> call methods that reproduce the problem.
>>>
>>> Adding a manual test to the JBS wouldn’t help, because nobody would 
>>> ever run this test. So the best is to make an automatic jtreg test, 
>>> or at least convert your manual tests to jtreg and push them into 
>>> the repo together with the fix.
>>>
>>> With best regards. Petr.
>> Discussed with SQE, auto test for this will require more 
>> investigation, since it's not only a test to get access information 
>> of a component,  in addition, it requires some human interaction, so 
>> an autotest is possible with combine Robot+JTreg, since this target 
>> to 14_03,  another bug JDK-8039978 is filed to track down the auto 
>> test requirement for this, it's a good point since I will have more 
>> of such fix coming up.
>>>
>>> 10 апр. 2014 г., в 6:04 после полудня, Vivi An <vivi.an at oracle.com> 
>>> написал(а):
>>>
>>>> Updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~dmarkov/8036983/jdk9/webrev.01/
>>>>
>>>> Comments as below
>>>>
>>>> Thanks
>>>>
>>>> Vivi
>>>>
>>>>
>>>>
>>>> On 4/9/2014 8:08 AM, Alexander Scherbatiy wrote:
>>>>> On 4/8/2014 9:19 PM, Vivi An wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you please review the fix for JDK 9?
>>>>>>
>>>>>> This bug is JAB related. ActivateDescenderPropertyChanged event 
>>>>>> for JTree and JTable were not sent properly in case 
>>>>>> SHIFT+CursorDown or Ctrl+CursorUp/Down are used. Fix made mainly 
>>>>>> uses lead path (acctive child path) instead of selection path to 
>>>>>> check if an event needs to be fired.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8036983
>>>>>> Webrev: http://cr.openjdk.java.net/~dmarkov/8036983/jdk9/webrev.00/
>>>>>>
>>>>> +        public void fireActiveDescendantPropertyChange(TreePath 
>>>>> oldPath, TreePath newPath) {
>>>>> Is it possible to make the method package access instead of public?
>>>> Yes, good idea, fixed
>>>>> +            int focusedRow = 
>>>>> JTable.this.getSelectionModel().getLeadSelectionIndex();;
>>>>> There is one more semicolon at the end.
>>>>>
>>>> Fixed
>>>>> Is it possible to write a test (manual or automated) for the fix?
>>>> Two test files (One for JTree, one for JTable) attached to the bug 
>>>> for manual test,  comments added in each file about the how to do 
>>>> the test.
>>>>> Thanks,
>>>>> Alexandr.
>>>>>> Thanks
>>>>>>
>>>>>> ~ Vivi
>>
>