Hi Sergey,

I updated the text based on your recommendations. Please find the new version 
here: http://cr.openjdk.java.net/~dmarkov/8225505/webrev.04/ 
<http://cr.openjdk.java.net/~dmarkov/8225505/webrev.04/>

Thanks,
Dmitry

> On 23 Aug 2019, at 00:00, Sergey Bylokhov <[email protected]> wrote:
> 
> On 8/20/19 2:18 am, Dmitry Markov wrote:
>> Any feedback regarding the test?
> 
> I guess you need to rephrase the text(to select the element by the keyboard) 
> because if the user
> will select the menu item by the mouse the tooltip will be visible even 
> before the fix.
> 
>> Thanks,
>> Dmitry
>>> On 20 Aug 2019, at 09:01, Prasanta Sadhukhan <[email protected] 
>>> <mailto:[email protected]> 
>>> <mailto:[email protected] 
>>> <mailto:[email protected]>>> wrote:
>>> 
>>> +1
>>> 
>>> Regards
>>> Prasanta
>>> On 19-Aug-19 8:44 PM, Dmitry Markov wrote:
>>>> Hi Prasanta,
>>>> 
>>>> Thank you for the review. I have added the regression test as you 
>>>> requested. Please find the new version here: 
>>>> http://cr.openjdk.java.net/~dmarkov/8225505/webrev.03/ 
>>>> <http://cr.openjdk.java.net/~dmarkov/8225505/webrev.03/>
>>>> 
>>>> Thanks,
>>>> Dmitry
>>>> 
>>>>> On 19 Aug 2019, at 10:12, Prasanta Sadhukhan 
>>>>> <[email protected] <mailto:[email protected]> 
>>>>> <mailto:[email protected] 
>>>>> <mailto:[email protected]>>> wrote:
>>>>> 
>>>>> Fix looks ok to me but I think a testcase is required, even if manual.
>>>>> 
>>>>> Regards
>>>>> Prasanta
>>>>> On 16-Aug-19 2:23 PM, Dmitry Markov wrote:
>>>>>> I still need a second reviewer. Any volunteers?
>>>>>> 
>>>>>> Thank you in advance,
>>>>>> Dmitry
>>>>>> 
>>>>>>> On 8 Aug 2019, at 08:37, Dmitry Markov <[email protected] 
>>>>>>> <mailto:[email protected]> <mailto:[email protected] 
>>>>>>> <mailto:[email protected]>>> wrote:
>>>>>>> 
>>>>>>> Thank you, Sergey!
>>>>>>> Looking for the second “+1”.
>>>>>>> 
>>>>>>> Dmitry
>>>>>>> 
>>>>>>>> On 8 Aug 2019, at 00:25, Sergey Bylokhov <[email protected] 
>>>>>>>> <mailto:[email protected]> <mailto:[email protected] 
>>>>>>>> <mailto:[email protected]>>> wrote:
>>>>>>>> 
>>>>>>>> Looks fine.
>>>>>>>> 
>>>>>>>> [email protected] <mailto:[email protected]> 
>>>>>>>> <mailto:[email protected] 
>>>>>>>> <mailto:[email protected]>>wrote:
>>>>>>>> >
>>>>>>>> > Hi Sergey,
>>>>>>>> >
>>>>>>>> > I looked into your suggestion again and found that it is possible to 
>>>>>>>> > use MenuKeyListener inside ToolTipManager without new event 
>>>>>>>> > generation. So I updated the fix based on your recommendation. 
>>>>>>>> > Please find the new version here: 
>>>>>>>> > http://cr.openjdk.java.net/~dmarkov/8225505/webrev.02/ 
>>>>>>>> > <http://cr.openjdk.java.net/~dmarkov/8225505/webrev.02/>
>>>>>>>> >
>>>>>>>> > Thanks,
>>>>>>>> > Dmitry
>>>>>>>> 
>>>>>>>>    > On 2 Aug 2019, at 23:57, Sergey Bylokhov 
>>>>>>>> <[email protected] <mailto:[email protected]> 
>>>>>>>> <mailto:[email protected] 
>>>>>>>> <mailto:[email protected]>>> wrote:
>>>>>>>> 
>>>>>>>>    >
>>>>>>>>    > Hi, Dmitry.
>>>>>>>> 
>>>>>>>>        Yes, it is possible to get the same result using the approach 
>>>>>>>> you
>>>>>>>>        mentioned. Unfortunately registration of MenuKeyListener inside
>>>>>>>>        ToolTipManager is not enough. Also we need to generate new
>>>>>>>>        MenuKeyEvent with proper source (component) to let the tooltip 
>>>>>>>> manager
>>>>>>>>        know for which menu element tooltip text should be 
>>>>>>>> displayed/hidden
>>>>>>>>        (similar thing I did in my proposal, see 
>>>>>>>> processToolTipKeyEvent()
>>>>>>>>        method).
>>>>>>>>        I am sorry but I do not think we have to implement such 
>>>>>>>> solution since
>>>>>>>>        generation of new events is still necessary plus implementation 
>>>>>>>> of
>>>>>>>>        MenuKeyListener is required.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>    Are you sure that the new events a necessary? It will be be really 
>>>>>>>> good to implement the fix w/o such events. We cannot sent keyEvents 
>>>>>>>> since we never do it before, and it will be good not to sent new 
>>>>>>>> MenuKeyEvent. Are you sure that an existed MenuKeyEvent event does not 
>>>>>>>> have enough information?(It has the component, path and current 
>>>>>>>> MenuSelectionManager).
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>> 
> 
> 
> -- 
> Best regards, Sergey.

Reply via email to