On Mon, 28 Apr 2025 20:03:24 GMT, Martin Fox <m...@openjdk.org> wrote:
>> Indeed... this test fails: >> `TableColumnHeaderTest::testColumnRightClickDoesAllowResizingWhenConsumed`. >> >> It has this JavaDoc: >> >> /** >> * When a right click is done on a table column header and consumed by a >> self added event handler, the column >> * drag lock should be set to true in the pressed handler, but still to >> false again in the released handler.<br> >> * By that we guarantee, that the column resizing still works. >> */ >> ``` >> This was changed based on @beldenfox suggestion. I'll change it back. > >> This was changed based on @beldenfox suggestion. I'll change it back. > > If the mouse pressed event triggers a context menu you might never see the > mouse released event. So you have to consider what it means if column drag > lock gets set and then never cleared. > > The failing test strikes me as odd, it is explicitly testing a bit of > internal state to verify that a right-click drag operation will work. Users > don't expect this to work and overloading right-click makes life complicated > so right-click drags are generally disabled even on Windows. I understand > that this test case was added because the table header was getting into a > persistent bad state but unless I'm missing something (and I certainly might > be) the bug should have been resolved by turning right-click drag into a > no-op. (It doesn't help that the test uses the MouseEventFirer which sets the > isPopupTrigger flag on both pressed and release which doesn't match the > behavior of any platform.) @beldenfox I understand your concerns about `columnDragLock` and the related tests `testColumnRightClickDoesAllowResizing()` and `testColumnRightClickDoesAllowResizingWhenConsumed()`, but if I get it right, this PR doesn't really modify the behaviour of such lock. So I suggest we take this on a follow up issue, revisiting the need to change the lock from true to false when the popup is triggered, and modifying or removing those tests accordingly. What do you think? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1754#issuecomment-2836995192
