Fix looks good to me. > On 14-Jun-2017, at 5:30 pm, swing-dev-requ...@openjdk.java.net wrote: > > Date: Wed, 14 Jun 2017 12:34:04 +0530 > From: Alexander Zvegintsev <alexander.zvegint...@oracle.com > <mailto:alexander.zvegint...@oracle.com>> > To: Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com > <mailto:prasanta.sadhuk...@oracle.com>>, Sergey > Bylokhov <sergey.bylok...@oracle.com > <mailto:sergey.bylok...@oracle.com>> > Cc: swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> > Subject: Re: <Swing Dev> [10] RFR: JDK-8043315: Nimbus: Setting > Nimbus.Overrides property affects custom keymap installation > Message-ID: <67b025e2-2134-a1fb-8769-078ff1f14...@oracle.com > <mailto:67b025e2-2134-a1fb-8769-078ff1f14...@oracle.com>> > Content-Type: text/plain; charset=utf-8; format=flowed > > Hi Prasanta, > >> If app again calls setKeymap(null) then the static variable will be >> "true" and it will reset back to default keymap. > It doesn't seem to be relevant to this fix. setKeymap(): " Setting to > <code>null</code> effectively disables keyboard input." As it does with > you fix. > > Otherwise the fix looks good to me. > > Thanks, > Alexander. > > On 02/06/2017 13:59, Prasanta Sadhukhan wrote: >> Hi Sergey, >> >> I have modified webrev to remove static keyword and made the test >> automated >> http://cr.openjdk.java.net/~psadhukhan/8043315/webrev.01/ >> <http://cr.openjdk.java.net/~psadhukhan/8043315/webrev.01/> >> Updated fix tests if app has fired a property change by calling >> setKeymap() then it will not uninstall custom keymap and let the >> custom keymap be honoured. If app again calls setKeymap(null) then the >> static variable will be "true" and it will reset back to default keymap. >> >> Regards >> Prasanta >> On 6/2/2017 12:07 AM, Sergey Bylokhov wrote: >>> Hi, Prasanta. >>> Can you please clarify the fix a little bit. >>> You have a static variable, which is set to "false" when the listener >>> for "keymap" is triggered, and it seems that you never set it back to >>> "true"? Is it intentional behavior? >>> Note that if there are a few objects of "SynthEditorPaneUI" then they >>> will share this static field. Also it seems that the test can be >>> automated, because currently it is requires from the user to press >>> only one button("space"). >>> >>> ----- prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com> >>> wrote: >>> >>>> Hi All, >>>> >>>> Please review a bug fix for Nimbus L&F where if app sets custom keymap >>>> >>>> and then sets Nimbus.Overrides property, >>>> then the custom keymap is not honoured. >>>> For example, if testapp sets a new action for "space" key press and >>>> sets >>>> sets Nimbus.Overrides property, >>>> it will not be honoured and default action ie. inserting a "space" >>>> character will be done. >>>> >>>> Issue was NimbusLookAndFeel#shouldUpdateStyleOnEvent() returns true >>>> for >>>> Nimbus.Override property which causes SynthEditorPaneUI#updateStyle() >>>> to >>>> be called which >>>> uninstall set keyboard actions and sets up default keyboard action. >>>> >>>> Proposed fix is to check if a keymap is already set (a propertyChange >>>> >>>> event will be fired for when app calls setKeyMap()) then do not reset >>>> to >>>> default keymap. >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8043315 >>>> <https://bugs.openjdk.java.net/browse/JDK-8043315> >>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8043315/webrev.00/ >>>> <http://cr.openjdk.java.net/~psadhukhan/8043315/webrev.00/> >>>> >>>> Regards >>>> Prasanta >> >
