Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8202296: Monocle MouseInput doesn't send keyboard modifiers in events. #170

Closed
wants to merge 5 commits into from

Conversation

@tomsontom
Copy link
Collaborator

@tomsontom tomsontom commented Apr 10, 2020

Extract keystate and add to the existing modifier mask, to support eg
multi-select

https://bugs.openjdk.java.net/browse/JDK-8202296


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8202296: Monocle MouseInput doesn't send keyboard modifiers in events.

Reviewers

  • Kevin Rushforth (kcr - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jfx pull/170/head:pull/170
$ git checkout pull/170

Extract keystate and add to the existing modifier mask, to support eg
multi-select
@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 10, 2020

Hi @tomsontom, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user tomsontom" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

@bridgekeeper bridgekeeper bot added oca and removed oca labels Apr 10, 2020
@openjdk openjdk bot added the rfr label Apr 14, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 14, 2020

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Apr 14, 2020

The fix looks simple enough. Can you add a unit test?

Added Test for keyboard modifiers
@openjdk openjdk bot removed the rfr label Apr 15, 2020
tomsontom added 3 commits Apr 15, 2020
Fix whitespace errors
Fix whitespace errors
Fix whitespace errors
Copy link
Member

@kevinrushforth kevinrushforth left a comment

Looks good to me. I verified that the new test fails without your fix and passes with your fix.

In case anyone else is looking at this and wants to run the test, it isn't enabled by default, so you need to run it like this:

gradle -PUNSTABLE_TEST=true -PFULL_TEST=true -PUSE_ROBOT=true \
    :systemTests:test --tests test.robot.com.sun.glass.ui.monocle.RobotTest
@openjdk
Copy link

@openjdk openjdk bot commented Apr 28, 2020

@tomsontom This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type /integrate in a new comment to proceed. After integration, the commit message will be:

8202296: Monocle MouseInput doesn't send keyboard modifiers in events.

Reviewed-by: kcr
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /solves command.

Since the source branch of this PR was last updated there have been 43 commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate c14cc44eaa5794225a9f896123c18338eb1ac915.

As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinrushforth) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready label Apr 28, 2020
@tomsontom
Copy link
Collaborator Author

@tomsontom tomsontom commented May 12, 2020

@kevinrushforth is there anything left todo or did you simply forget to merge it?

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented May 12, 2020

@tomsontom Yes, there is something you need to do. As the author of the patch, you need to /integrate it after which I'll /sponsor it.

@tomsontom
Copy link
Collaborator Author

@tomsontom tomsontom commented May 12, 2020

/integrate

@openjdk
Copy link

@openjdk openjdk bot commented May 12, 2020

@tomsontom
Your change (at version 8fe96a9) is now ready to be sponsored by a Committer.

@openjdk openjdk bot added the sponsor label May 12, 2020
@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented May 12, 2020

/sponsor

@openjdk openjdk bot closed this May 12, 2020
@openjdk openjdk bot added integrated and removed sponsor ready rfr labels May 12, 2020
@openjdk
Copy link

@openjdk openjdk bot commented May 12, 2020

@kevinrushforth @tomsontom The following commits have been pushed to master since your change was applied:

  • c14cc44: 8244417: support static build for Windows
  • b14e085: 8244735: Error on iOS passing keys with unicode values greater than 255
  • b0d66d0: 8242508: Upgrade to Visual Studio 2019 version 16.5.3
  • 0f87d20: 8244487: One Windows 10 SDK file missing from FX build
  • 4ec163d: 8242001: ChoiceBox: must update value on setting SelectionModel, part2
  • 236e2d6: 8244421: Wrong scrollbar position on touch enabled devices
  • 0385563: 8243255: Font size is large in JavaFX app with enabled Monocle on Raspberry Pi
  • 2e90076: 8242507: Add support for Visual Studio 2019
  • 39d9c3b: 8244110: NPE in MenuButtonSkinBase change listener
  • 99f7747: 8241999: ChoiceBox: incorrect toggle selected for uncontained
  • 1cae6a8: 8176499: Dependence on java.util.Timer freezes screen when OS time resets backwards
  • 2b9eb52: 8237504: Update copyright header for files modified in 2020
  • 3130fc8: 8198402: ToggleButton.setToggleGroup causes memory leak when button is removed via ToggleGroup.getToggles()
  • 8ad5805: 8191758: Match WebKit's font weight rendering with JavaFX
  • 66c3b38: 8227425: Add support for e-paper displays on i.MX6 devices
  • e30049f: 8242077: Add information about HTTP/2 and HttpClient usage in WebEngine
  • e0ffca3: 8242505: Some WebKit tests might fail because Microsoft libraries are not loaded
  • ceb3fce: 8087555: [ChoiceBox] uncontained value not shown
  • 818ac00: 8175358: Memory leak when moving MenuButton into another Scene
  • 91d4c8b: 8241737: TabPaneSkin memory leak on replacing selectionModel
  • 48476eb: 8241582: Infinite animation does not start from the end when started with a negative rate
  • dedf7cb: 8242490: Upgrade to gcc 9.2 on Linux
  • 5e9fb82: 8242577: Cell selection fails on iOS most of the times
  • 69e4266: 8242489: ChoiceBox: initially toggle not sync'ed to selection
  • 1d88180: 8243112: Skip failing test SVGTest.testSVGRenderingWithPattern
  • ec8608f: 8223298: SVG patterns are drawn wrong
  • e82046e: 8242530: [macos] Some audio files miss spectrum data when another audio file plays first
  • 7044cef: 8241476: Linux build warnings issued on gcc 9
  • 9d50c4c: Merge
  • 4d69a0d: 8241629: [macos10.15] Long startup delay playing media over https on Catalina
  • b1fdc45: 8242209: Increase web native thread stack size for x86 mode
  • ef37669: Merge
  • f2bca9f: Merge
  • 6900d29: Merge
  • e91bec4: Merge
  • 66a8f49: Merge
  • fde42da: Merge
  • e21fd1f: Merge
  • 443c845: Merge
  • 31e63de: Merge
  • 14c6938: 8236798: Enhance FX scripting support
  • bfb2d0e: Merge
  • 39f6127: Merge

Your commit was automatically rebased without conflicts.

Pushed as commit 435671e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants