mattcasters commented on code in PR #8530:
URL: https://github.com/apache/hop/pull/8530#discussion_r4153823814
##########
ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java:
##########
@@ -766,6 +766,16 @@ private void enableToolbarButtons(int nrRows) {
private MouseListener createTableMouseListener() {
return new MouseAdapter() {
+ // A single click selects the row in a read-only grid, so double-click
is what opens the
+ // view-only editor: the one place a long or multi-line value can be
read in full and its
+ // text selected.
+ @Override
+ public void mouseDoubleClick(MouseEvent event) {
+ if (readonly && event.button == 1 && activeTableItem != null &&
activeTableColumn > 0) {
Review Comment:
**[suggestion]** The handler never checks that the event is inside the
active cell. It trusts `activeTableRow` / `activeTableColumn`. `mouseDown`
returns without updating those fields when the click is to the right of the
last column, and a click below the last row does `setPosition(itemCount - 1,
1)` before `insertRowAfter()` (a no-op while read-only). The double-click is
still delivered, so this opens an editor on a stale cell or on the last row's
first column.
**Suggestion:** Hit-test the point the same way `mouseDown` does, or require
it to be inside `activeTableItem.getBounds(activeTableColumn)`, and return
otherwise.
##########
ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java:
##########
@@ -766,6 +766,16 @@ private void enableToolbarButtons(int nrRows) {
private MouseListener createTableMouseListener() {
return new MouseAdapter() {
+ // A single click selects the row in a read-only grid, so double-click
is what opens the
+ // view-only editor: the one place a long or multi-line value can be
read in full and its
+ // text selected.
+ @Override
+ public void mouseDoubleClick(MouseEvent event) {
+ if (readonly && event.button == 1 && activeTableItem != null &&
activeTableColumn > 0) {
+ edit(activeTableRow, activeTableColumn);
Review Comment:
**[bug]** `mouseDoubleClick` calls `edit()` for every `readonly` table, and
the comment says that opens the view-only editor. `editText` sets `viewOnly`
only from `colinfo.isReadOnly()`, and `editMultiline` does the same.
`focusLost` and `applyTextChange` skip the write only in `isColumnReadOnly`.
`TableView.readonly` does not.
Grids that set only the table flag still get a normal editor.
`DatabaseTableInfoTab` builds plain text columns (not read-only) and then
`setReadonly(true)`. `BaseDialog.applyReadOnlyControls` does the same for every
`TableView` in a read-only metadata dialog. Double-click opens a text, combo,
or multi-line editor; leaving the cell runs `row.setText` / `setCellValue` and
the modify listener. That undoes the single-click guard just added in
`editSelected`. SQL results are safe only because
`RowPreviewSupport.applyColumnMeta` also marks each column read-only.
**Suggestion:** Treat `readonly` as view-only inside `editText`,
`editCombo`, and `editMultiline` (`SWT.READ_ONLY`, no modify listeners, no
write-back), the same as a read-only column. That also covers Enter, F2, and
typing, which still call `edit()` with no `readonly` check. If the pop-out
should open only for real view-only cells, gate this handler with
`isColumnReadOnly(activeTableColumn)` instead of calling `edit()`
unconditionally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
