8236839: System menubar removed when other menubars are created or modified by jkaving · Pull Request #86 · openjdk/jfx

jkaving · 2020-01-14T19:50:28Z

This pull request fixes the sceneProperty listener in MenuBarSkin so that we leave the
current system menubar alone when other menubars are changed.

It also adds a test case that reproduces the problem before the fix.

Progress

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

Issue

JDK-8236839: System menubar removed when other menubars are created or modified

Approvers

Kevin Rushforth (kcr - Reviewer)
Ajit Ghaisas (aghaisas - Reviewer)

In the sceneProperty listener in MenuBarSkin we now leave the current system menubar alone if the stage that was changed is not the one that has the system menubar. Also add a test case that reproduces the problem caused by the previous behavior. Fixes JDK-8236839

bridgekeeper · 2020-01-14T19:51:36Z

Hi jkaving, 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 jkaving" 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.

jkaving · 2020-01-14T19:53:41Z

/signed

bridgekeeper · 2020-01-14T19:54:40Z

Thank you! Please allow for up to two weeks to process your OCA, although it is usually done within one to two business days. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

jkaving · 2020-01-14T20:06:58Z

I just realised that I did not make the commit with the same email address that I used on the OCA (but both addresses are linked to my GitHub account).
Will this be a problem?
Should I redo the commit with the correct email address and do a force push to my branch?

kevinrushforth · 2020-01-14T20:30:39Z

No, it's not a problem. The commit email address doesn't matter as long as your OCA can be validated.

mlbridge · 2020-01-20T14:53:00Z

Webrevs

kevinrushforth · 2020-01-28T18:28:35Z

/reviewers 2

openjdk · 2020-01-28T18:28:42Z

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

kevinrushforth · 2020-01-28T18:29:33Z

@aghaisas can be the second reviewer.

kevinrushforth

This looks good to me. I confirm that this fixes the bug, and that the new unit test catches the problem.

I added a minor comment about the import statement.

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/MenuBarSkinTest.java

kevinrushforth

Looks good.

openjdk · 2020-02-07T10:53:17Z

@jkaving 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:

8236839: System menubar removed when other menubars are created or modified

Reviewed-by: kcr, aghaisas

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 44 commits pushed to the master branch. Since 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 21d3b7e036cabeaf4bfd7ae72ce69e9e3bc27ae3.

As you are not a known OpenJDK Author, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinrushforth, @aghaisas) 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).

jkaving · 2020-02-07T14:27:50Z

/integrate

openjdk · 2020-02-07T14:36:08Z

@jkaving
Your change (at version 811035a) is now ready to be sponsored by a Committer.

kevinrushforth · 2020-02-07T14:38:40Z

/sponsor

openjdk · 2020-02-07T14:43:48Z

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

21d3b7e: 8237453: [TabPane] Incorrect arrow key traversal through tabs after reordering
e224e54: 8238526: Cherry pick GTK WebKit 2.26.3 changes
6968e38: 8237469: Inherited styles don't update when node is moved
3c55a7e: Merge
a4b9f24: 8237503: Update copyright header for files modified in 2020
1749e85: 8237975: Non-embedded Animations do not play backwards after being paused
2ab40c1: 8231513: JavaFX cause Keystroke Receiving prompt on MacOS 10.15 (Catalina)
aa91ebb: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
56267e1: 8237770: Error creating fragment phong shader on iOS
95bf2c0: 8237782: Only read advances up to the minimum of the numHorMetrics or the available font data.
d05e8fc: 8237833: Check glyph size before adding to glyph texture cache
1213ea7: Merge
1823f6e: 8088198: Exception thrown from snapshot if dimensions are larger than max texture size
ca37c1f: 8238249: GetPrimitiveArrayCritical passed with hardcoded FALSE value
b96bc52: 8237003: Remove hardcoded WebAnimationsCSSIntegrationEnabled flag in DumpRenderTree
d303a21: Merge
5a0e71b: 8237372: NullPointerException in TabPaneSkin.stopDrag
79fc0d0: 8232824: Removing TabPane with strong referenced content causes memory leak from weak one
aa6f3a4: 8236912: NullPointerException when clicking in WebView with Button 4 or Button 5
921389f: Merge
da99e24: 8237823: Mark TextTest.testTabSize as unstable
66ac99f: Merge
9ae37f1: 8236753: Animations do not play backwards after being stopped
b2d8564: 8233942: Update to 609.1 version of WebKit
f5ee963: 8157224: isNPOTSupported check is too strict
be22e85: 8237078: [macOS] Media build broken on XCode 11
20325e1: Merge
16cea41: Merge
63520a0: Merge
8a5344c: Merge
2e0d01c: Merge
a96704e: Merge
71fa9af: Merge
a3711e2: Merge
2d5d7e0: Merge
962bdd1: 8232214: Improved internal validations
2d2b824: 8232128: Better formatting for numbers
81f7738: 8232121: Better numbering system
8e55294: Merge
1325f11: Merge
f759dd9: Merge
fed185a: Merge
592d745: Merge
a0b4b14: 8227473: Improve gstreamer media support

Your commit was automatically rebased without conflicts.

Pushed as commit a74137a.

mlbridge · 2020-02-07T14:45:09Z

Mailing list message from Kevin Rushforth on openjfx-dev:

Changeset: a74137a
Author: Johan Kaving <johan.kaving at looklet.com>
Committer: Kevin Rushforth <kcr at openjdk.org>
Date: 2020-02-07 14:43:29 +0000
URL: https://git.openjdk.java.net/jfx/commit/a74137a1

8236839: System menubar removed when other menubars are created or modified

Reviewed-by: kcr, aghaisas

! modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
! modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/MenuBarSkinTest.java

bridgekeeper bot added the oca Needs verification of OCA signatory status label Jan 14, 2020

bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label Jan 14, 2020

bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels Jan 20, 2020

openjdk bot added the rfr Ready for review label Jan 20, 2020

kevinrushforth self-requested a review January 28, 2020 18:28

kevinrushforth assigned jkaving Jan 28, 2020

kevinrushforth reviewed Feb 5, 2020

View reviewed changes

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/MenuBarSkinTest.java Outdated Show resolved Hide resolved

openjdk bot removed the rfr Ready for review label Feb 6, 2020

Move import statement

811035a

jkaving force-pushed the JDK-8236839 branch from 94b2dba to 811035a Compare February 6, 2020 13:21

openjdk bot added the rfr Ready for review label Feb 6, 2020

kevinrushforth approved these changes Feb 6, 2020

View reviewed changes

aghaisas approved these changes Feb 7, 2020

View reviewed changes

openjdk bot added the ready Ready to be integrated label Feb 7, 2020

openjdk bot added the sponsor Ready to sponsor label Feb 7, 2020

openjdk bot closed this Feb 7, 2020

openjdk bot added integrated Pull request has been integrated and removed sponsor Ready to sponsor ready Ready to be integrated rfr Ready for review labels Feb 7, 2020

Conversation

jkaving commented Jan 14, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Approvers

Uh oh!

bridgekeeper bot commented Jan 14, 2020

Uh oh!

jkaving commented Jan 14, 2020

Uh oh!

bridgekeeper bot commented Jan 14, 2020

Uh oh!

jkaving commented Jan 14, 2020

Uh oh!

kevinrushforth commented Jan 14, 2020

Uh oh!

mlbridge bot commented Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

kevinrushforth commented Jan 28, 2020

Uh oh!

openjdk bot commented Jan 28, 2020

Uh oh!

kevinrushforth commented Jan 28, 2020

Uh oh!

kevinrushforth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevinrushforth left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkaving commented Feb 7, 2020

Uh oh!

openjdk bot commented Feb 7, 2020

Uh oh!

kevinrushforth commented Feb 7, 2020

Uh oh!

openjdk bot commented Feb 7, 2020

Uh oh!

mlbridge bot commented Feb 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

jkaving commented Jan 14, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 20, 2020 •

edited

Loading

openjdk bot commented Feb 7, 2020 •

edited

Loading