On Mon, 28 Apr 2025 18:27:37 GMT, Jose Pereda <jper...@openjdk.org> wrote:

>> Note: The JBS issue 
>> [JDK-8207333](https://bugs.openjdk.org/browse/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.
>
> Jose Pereda has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8207333-contextmenusort
>  - remove white space
>  - keep column drag lock
>  - Prevent drag or start sorting on if popup is triggered, test ctrl+left 
> click on macOS
>  - Check popupTrigger on mouse pressed too for tableColumnHeaders

I think this PR should probably be accepted as-is. It fixes an obvious problem 
and doesn't introduce any new ones. Someone should enter a follow-up PR citing 
some of the other problems but most of those have been around for years and 
only affect users who are looking for trouble (most wouldn't even consider 
using the right mouse button to drag).

Here you have separate behavior on click vs drag-click and have to deal with 
two different approaches to context menus. I've tackled this sort of thing 
before and it usually takes two or four or six tries to get right. To reduce 
the complexity you have be aggressive at ignoring irrelevant events (no 
right-click drag!) and avoid setting up state related to dragging until the 
user actually starts dragging. So this code needs a modest re-write but, again, 
it doesn't need to happen in this PR.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1754#issuecomment-2839339848

Reply via email to