8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

[johanvos]

Don't set TabHeader to mouseTransparent, since it might get stuck in that state (e.g. in case an Alert is shown).
The TabPaneSkin deals with the dragging internally, and does not require the dragged node to be mouseTransparent.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

Reviewers

• Ambarish Rapte (@arapte - Reviewer)
• Kevin Rushforth (@kevinrushforth - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/895/head:pull/895
$ git checkout pull/895

Update a local copy of the PR:
$ git checkout pull/895
$ git pull https://git.openjdk.org/jfx pull/895/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 895

View PR using the GUI difftool:
$ git pr show -t 895

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/895.diff

[bridgekeeper]

👋 Welcome back jvos! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[mlbridge]

Webrevs

• 00: Full (599e7c06)

[arapte]

The fix looks good to me. Testing looks good.
But I am not sure why I had included this call at the time of implementation.
Requesting @kevinrushforth to take a look.

[kevinrushforth]

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[johanvos]

The fix looks good to me. Testing looks good. But I am not sure why I had included this call at the time of implementation. Requesting @kevinrushforth to take a look.

I was wondering about the original reason to include these calls. I think it may be because of the javadoc for MouseDragEvent (https://openjfx.io/javadoc/19/javafx.graphics/javafx/scene/input/MouseDragEvent.html):

You can achieve this by calling setMouseTransparent(true) on the dragged node in a MOUSE_PRESSED handler and returning it back to false in a MOUSE_RELEASED handler.

This scenario doesn't take into account the case where for some reason a MOUSE_RELEASED event is never received on the specific node -- which is the case when an Alert is being shown.

[kevinrushforth]

The fix looks fine to me. I tested it and all is as expected.

I also don't remember the reason for the initial setting of mouseTransparent, but it was in the initial implementation of draggable tabs JDK-8187074. Johan's guess seems plausible.

[kevinrushforth]

@arapte can you also review?

[arapte]

Apologies for the delay.
Looks good to me. Tested on all three platforms.

[openjdk]

@johanvos This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

Reviewed-by: arapte, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 35 new commits pushed to the master branch:

• 086dac0: 8297213: Robot capture tests should move mouse to corner of screen
• e3e0dfd: 8297166: [TestBug] Fix some ignored unit test from TableViewTest
• 626227d: 8296330: Tests: Add layout container snapping tests
• 1ec06a8: 8206430: Use consistent pattern for startup in FX system tests
• 74af45d: 8296556: Mark two RobotTest methods as unstable on HiDPI Windows systems
• 6cc9c4d: 8297042: gradle -PBUILD_SDK_FOR_TEST=false fails with gradle 7.6
• a13630f: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin
• 7ec0852: 8291853: [CSS] ClassCastException in CssStyleHelper calculateValue
• 5956950: 8285503: SwingNodeDnDMemoryLeakTest::testSwingNodeMemoryLeak fails sometimes
• 0ee116d: 8296589: PixelBufferDrawTest fails on some systems
• ... and 25 more: https://git.openjdk.org/jfx/compare/28f8fa9c20363ced9a94787ecfd45735b3e6b82e...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[johanvos]

/integrate

[openjdk]

Going to push as commit cce8580.
Since your change was applied there have been 37 commits pushed to the master branch:

• a0ea874: 8279214: Memory leak in Scene after dragging a cell
• 9819d45: 8297332: Remove easy warnings in javafx.base
• 086dac0: 8297213: Robot capture tests should move mouse to corner of screen
• e3e0dfd: 8297166: [TestBug] Fix some ignored unit test from TableViewTest
• 626227d: 8296330: Tests: Add layout container snapping tests
• 1ec06a8: 8206430: Use consistent pattern for startup in FX system tests
• 74af45d: 8296556: Mark two RobotTest methods as unstable on HiDPI Windows systems
• 6cc9c4d: 8297042: gradle -PBUILD_SDK_FOR_TEST=false fails with gradle 7.6
• a13630f: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin
• 7ec0852: 8291853: [CSS] ClassCastException in CssStyleHelper calculateValue
• ... and 27 more: https://git.openjdk.org/jfx/compare/28f8fa9c20363ced9a94787ecfd45735b3e6b82e...master

Your commit was automatically rebased without conflicts.

[openjdk]

@johanvos Pushed as commit cce8580.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.