8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header #1754

jperedadnr · 2025-04-01T17:31:14Z

Note: The JBS issue JDK-8207333 refers to Linux, but it happens on macOS too.

When a TableColumn has a ContextMenu, if the user right clicks on the tableColumnHeader, the ContextMenu shows up, but, as described on the issue, on macOS and Linux, the table gets sorted as well.

Currently, TableColumnHeader#mouseReleasedHandler checks for MouseEvent::isPopupTriggered, but it doesn't have a check on mousePressed. However, it can be seen that a right click on the header has the following values for MouseEvent:: isPopupTriggered on the different platforms and mouse pressed and released events:

isPopupTriggered on:	Windows	macOS	Linux
mousePressed	false	true	true
mouseReleased	true	false	false

Also, the JavaDoc for MouseEvent::isPopupTriggered clearly states that:

Note: Popup menus are triggered differently on different systems. Therefore, popupTrigger should be checked in both onMousePressed and mouseReleased for proper cross-platform functionality.

Therefore, this PR adds a check to MouseEvent::isPopupTrigger in the mouse pressed event, that can be used later on to cancel the header sorting when the mouse released event happens.

Also a system test has been added. It fails on macOS and Linux, and passes on Windows before this PR, and passes on the three platforms with this PR.

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-8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header (Bug - P4)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1754

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-04-01T17:32:02Z

👋 Welcome back jpereda! 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.

openjdk · 2025-04-01T17:32:20Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

mlbridge · 2025-04-01T17:35:33Z

Webrevs

00: Full (612e345a)

andy-goryachev-oracle · 2025-04-01T17:39:12Z

I'll take a look.
Should we update the JBS and this PR title to remove [linux] as it applies to both platforms?

andy-goryachev-oracle · 2025-04-01T17:54:23Z

I can only test it on macOS and Windows. Could someone with a linux system help please?
/reviewers 2

openjdk · 2025-04-01T17:55:26Z

@andy-goryachev-oracle
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).

andy-goryachev-oracle

thank you for fixing this bug, @jperedadnr !

testing with the monkey tester looks good on macOS 15.3.2 and windows 11.

andy-goryachev-oracle · 2025-04-07T22:48:44Z

@Ziad-Mid could you be the second reviewer?

Ziad-Mid · 2025-04-08T10:53:24Z

@Ziad-Mid could you be the second reviewer?

I can't test on Linux , I will check same for macOS

beldenfox · 2025-04-08T16:55:51Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java

@@ -243,6 +245,7 @@ public TableColumnHeader(final TableColumnBase tc) {
        }

        if (me.isConsumed()) return;
+        popupTriggeredOnMousePressed = me.isPopupTrigger();
        me.consume();

        header.getTableHeaderRow().columnDragLock = true;


On Mac a context menu can be invoked using the right button or the left button in conjunction with the Control key. In the second case the pop trigger flag will be set and the primary button will be down. If I'm reading the code correctly this could get the header stuck in column re-ordering mode.

Testing on macOS 15.3.2, it does not get stuck when using control+left-click, works as expected.

There is one (existing and unrelated) problem - if the user tries to dismiss the popup menu by clicking on the header, it dismisses the popup (correctly), but then proceeds to re-sort the table (unexpected).

One can say it works as designed, since the click falls on a header cell and the expected operation is to toggle the sorting, with the side effect of dismissing the popup.

On the other hand, Excel (both mac and windows) behaves differently - a click on the spreadsheet header dismisses the popup and does not select the column, as it normally does when no popup is present.

What do you guys think?

control+left-click works as expected for me too.

When the context menu is showing, clicking anywhere outside of it, closes the popup (because of the autohide property), but doesn't consume the event. If you click on a button, for instance, it will be fired. In any case, this is the same before and after this PR, so maybe an issue for a later discussion.

definitely an unrelated issue, I just wanted to ask whether you guys think it crosses the threshold for a bug.

control+left-click works as expected for me too.

For control+left-click it looks like the code will call columnReordingStarted but on the released event it won't call columnReorderingComplete. I have no idea if this will lead to problems or not.

beldenfox · 2025-04-08T16:56:07Z

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java

@@ -271,7 +274,10 @@ public TableColumnHeader(final TableColumnBase tc) {
        TableColumnHeader header = (TableColumnHeader) me.getSource();
        header.getTableHeaderRow().columnDragLock = false;


On Ubuntu 22.04 the table column header is not receiving a mouse released event if a context menu is shown. Not sure where the released event is going but it's not to any node on the primary stage. The docs are vague on this but this looks like a separate bug that's making it hard to test this PR.

what happens in the master branch (without this PR's changes)?

I was having trouble reproducing the original bug on the master branch; the sorting is done when the mouse is released and most of the time that event never arrives (I think it does every now and then).

this is strange - the bug is immediately apparent. which macOS version do you have? although I don't think the version matters.

I am testing with an external mouse, with control-left-click, and with the double-finger-click gesture on the trackpad.

I was having problems reproducing on Linux. No problem reproducing on macOS.

On Linux I have some issues too: the test that I attached to the JBS issue doesn't fail for me on Linux, but the test that I added to this PR does fail without the fix.

Can you try the monkey tester? it allows setting properties on the table view / table column at runtime...

I just verified that on Linux the released event usually goes to the popup window. Basically it's whatever window is under the cursor at the time; if I move the mouse fast enough it may go to a node on the primary stage. I don't think this is our doing, it's how GTK is delivering the event.

On the JavaFX side I'm not sure this counts as a bug. The docs state that entered and released events should arrive in pairs during drag and drop operations but doesn't address this case.

The Monkey tester app works for me on Linux (Ubuntu 22.04, either mouse or pad), before and after this PR (so I can't reproduce the issue there, as I mentioned, though the test added to the PR fails without the patch).

Check popupTrigger on mouse pressed too for tableColumnHeaders

612e345

openjdk bot added the rfr Ready for review label Apr 1, 2025

andy-goryachev-oracle approved these changes Apr 1, 2025

View reviewed changes

jperedadnr changed the title ~~8207333: [linux] Column sorting is triggered always after context menu request on table header~~ 8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header Apr 1, 2025

beldenfox reviewed Apr 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header #1754

8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header #1754

jperedadnr commented Apr 1, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 1, 2025

openjdk bot commented Apr 1, 2025

mlbridge bot commented Apr 1, 2025

andy-goryachev-oracle commented Apr 1, 2025

andy-goryachev-oracle commented Apr 1, 2025

openjdk bot commented Apr 1, 2025

andy-goryachev-oracle left a comment

andy-goryachev-oracle commented Apr 7, 2025 •

edited

Loading

Ziad-Mid commented Apr 8, 2025

beldenfox Apr 8, 2025

andy-goryachev-oracle Apr 8, 2025

jperedadnr Apr 8, 2025

andy-goryachev-oracle Apr 8, 2025

beldenfox Apr 8, 2025

beldenfox Apr 8, 2025

andy-goryachev-oracle Apr 8, 2025

beldenfox Apr 8, 2025

andy-goryachev-oracle Apr 8, 2025

beldenfox Apr 8, 2025

jperedadnr Apr 8, 2025

andy-goryachev-oracle Apr 8, 2025

beldenfox Apr 8, 2025

jperedadnr Apr 9, 2025

		@@ -271,7 +274,10 @@ public TableColumnHeader(final TableColumnBase tc) {
		TableColumnHeader header = (TableColumnHeader) me.getSource();
		header.getTableHeaderRow().columnDragLock = false;

8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header #1754

Are you sure you want to change the base?

8207333: [Linux, macOS] Column sorting is triggered always after context menu request on table header #1754

Conversation

jperedadnr commented Apr 1, 2025 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Apr 1, 2025

openjdk bot commented Apr 1, 2025

mlbridge bot commented Apr 1, 2025

Webrevs

andy-goryachev-oracle commented Apr 1, 2025

andy-goryachev-oracle commented Apr 1, 2025

openjdk bot commented Apr 1, 2025

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

andy-goryachev-oracle commented Apr 7, 2025 • edited Loading

Ziad-Mid commented Apr 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jperedadnr commented Apr 1, 2025 •

edited by openjdk bot

Loading

andy-goryachev-oracle commented Apr 7, 2025 •

edited

Loading