RFR: 8233040: ComboBoxPopupControl: remove eventFilter for F4

Ajit Ghaisas aghaisas at openjdk.org
Tue Nov 5 11:12:26 UTC 2019


On Tue, 5 Nov 2019 10:38:24 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

> On Mon, 4 Nov 2019 12:46:57 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:
> 
>> On Thu, 31 Oct 2019 15:04:35 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:
>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8233040
>>> 
>>> The issue is that the eventFilter in CBPC uses F4 for toggling the popup _and_ consumes it. This prevents client code to register there own filter. 
>>> 
>>> The fix was to remove the block entirely - F4 is already handled by a keyMapping in ComboBoxBehaviorBase. Added test that fails before the change and passes after, old tests are all passing.
>>> 
>>> ----------------
>>> 
>>> Commits:
>>>  - 2f015406: Fix: ComboBoxPopupControl - remove eventFilter for F4
>>> 
>>> Changes: https://git.openjdk.java.net/jfx/pull/28/files
>>>  Webrev: https://webrevs.openjdk.java.net/jfx/28/webrev.00
>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8233040
>>>   Stats: 159 lines in 2 files changed: 153 ins; 6 del; 0 mod
>>>   Patch: https://git.openjdk.java.net/jfx/pull/28.diff
>>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/28/head:pull/28
>> 
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboSpecialKeyTest.java line 1:
>> 
>>> 1: /*
>>> 2:  * Created on 30.10.2019
>> 
>> This file needs to have a standard copyright header. Can be found in other test files.
>> 
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxPopupControl.java line 564:
>> 
>>> 563:                 textField.fireEvent(ke);
>>> 564:             }
>>> 565:         } else if (ke.getCode() == KeyCode.F10 || ke.getCode() == KeyCode.ESCAPE) {
>> 
>> Remove reference to "F4 key" in a comment present in this method.
> 
> Ajit,
> 
> thanks for your review - changes pushed as suggested :)
> 
> Noticed that I forgot the copyright headers in my other pull requests as well - should I add them now or wait until they are in review?

It looks good to go. I think, one reviewer should suffice for this.

For other PRs, we can deal with missing copyright headers as we review them.

PR: https://git.openjdk.java.net/jfx/pull/28


More information about the openjfx-dev mailing list