GUACAMOLE-1740: Don't display clipboard contents in the clipboard editor until it is focused on. by aleitner · Pull Request #796 · apache/guacamole-client

aleitner · 2023-02-22T18:50:13Z

When bringing up the Guacamole menu (Ctrl+Alt+Shift) the contents of your clipboard are immediately visible within the clipboard editor. For privacy purposes, the contents of the clipboard editor should be hidden/collapsed until the user chooses to expand/show them.

This Pull request updates the Clipboard Directive to prevent displaying clipboard data in the clipboard editor until the clipboard editor has been focused on.

jmuehlner · 2023-02-22T19:08:33Z

Can you post a screenshot showing what this change would look like to a user?

jmuehlner · 2023-02-22T19:12:07Z

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

            // If the clipboard data is a string, render it as text
            if (typeof data.data === 'string')
-                element.value = data.data;
+                element.value = (shouldDisplay) ? data.data : '';


It looks like this just shows an empty clipboard until the user clicks on the clipboard element, and then the data pops in, yeah? It'd probably be lot more obvious to users if there was some sort of styling to indicate an inactive clipboard, and some indication to the user about what to do if they want to see the clipboard.

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

jmuehlner · 2023-02-22T21:54:59Z

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

        });

+        // Update the prefocus clipboard editor message
+        guacTranslate('CLIENT.TEXT_CLIPBOARD_AWAITING_FOCUS', '').then(


It might be a bit cleaner/easier to just do the translation in the template. That would also improve separation of concerns - the controller shouldn't have to care about what specific messages are being displayed to the user. All it should need to know is what the state of the clipboard is, exposing that state to the template which can then render different elements (with translated text in them) according to that state.

Also, what if a user literally has the text "Focus here to view clipboard data..." in their clipboard?

Having some quick visual method to clearly identify that the clipboard is inactive would be nice. Rather than just sticking a message into the textarea - how about visually disabling (greying out, etc) the textarea and rendering a message in a different spot where it couldn't be confused with actual clipboard contents?

jmuehlner · 2023-02-22T21:57:18Z

guacamole/src/main/frontend/src/translations/en.json

        "TEXT_USER_LEFT"                  : "{USERNAME} has left the connection.",
        "TEXT_RECONNECT_COUNTDOWN"        : "Reconnecting in {REMAINING} {REMAINING, plural, one{second} other{seconds}}...",
        "TEXT_FILE_TRANSFER_PROGRESS"     : "{PROGRESS} {UNIT, select, b{B} kb{KB} mb{MB} gb{GB} other{}}",
+        "TEXT_CLIPBOARD_AWAITING_FOCUS"   : "Focus here to view clipboard data...",


"Focus" seems like an excessively technical term here: one that people not fluent in web technologies might not understand. If the required action is to click on the clipboard, it should probably just say that plainly.

aleitner · 2023-02-24T17:43:07Z

Edited to remove branding from screenshots

jmuehlner · 2023-02-24T17:50:08Z

That looks pretty good! It does look like that screenshot has a branding extension applied however - can you show what it looks like without the extension?

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

guacamole/src/main/frontend/src/app/clipboard/templates/guacClipboard.html

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

jmuehlner · 2023-02-27T20:26:25Z

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

-         * user interface provided by this directive.
+         * user interface provided by this directive. We populate the clipboard
+         * editor via this DOM element rather than updating a model so that we
+         * have support for rich text contents.


I think we're getting a bit ahead of ourselves claiming that we "have support for rich text contents". Might want to clarify that this is to enable future support for this functionality, since we certainly don't have it now.

jmuehlner · 2023-02-27T20:33:27Z

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

         * determined from the HTML contents of the clipboard field.
         */
-        var updateClipboardData = function updateClipboardData() {
+        const updateClipboardData = function updateClipboardData() {


I'm a fan of using const over var for this sort of thing for sure - but since no actual changes were made to this function, we should just leave it unchanged as much as possible - to ensure clean git history. Same with the removal of line 105 below. Let's just leave things the way they were.

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js

…tor until it is focused on.

aleitner changed the title ~~KCM-186: Don't display clipboard contents in the clipboard editor until it is focused on.~~ GUAC-XXXX: Don't display clipboard contents in the clipboard editor until it is focused on. Feb 22, 2023

aleitner changed the title ~~GUAC-XXXX: Don't display clipboard contents in the clipboard editor until it is focused on.~~ GUACAMOLE-1740: Don't display clipboard contents in the clipboard editor until it is focused on. Feb 22, 2023

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from d67e99b to 36f9afd Compare February 22, 2023 18:59

jmuehlner reviewed Feb 22, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 22, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 22, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch 3 times, most recently from fabec83 to 1559de4 Compare February 22, 2023 21:38

jmuehlner reviewed Feb 22, 2023

View reviewed changes

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch 2 times, most recently from 8a0f410 to 7e3e990 Compare February 24, 2023 17:41

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from 7e3e990 to 57ed1a9 Compare February 24, 2023 18:16

jmuehlner reviewed Feb 24, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 24, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 24, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 24, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 24, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch 2 times, most recently from 7865795 to fe614eb Compare February 27, 2023 17:50

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from fe614eb to 515c043 Compare February 27, 2023 18:25

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from 515c043 to d6496eb Compare February 27, 2023 18:45

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from d6496eb to 7124870 Compare February 27, 2023 18:51

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Show resolved Hide resolved

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/templates/guacClipboard.html Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from 7124870 to be6445e Compare February 27, 2023 20:20

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from be6445e to 46b8145 Compare February 27, 2023 20:26

jmuehlner reviewed Feb 27, 2023

View reviewed changes

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from 46b8145 to dc3f786 Compare February 27, 2023 20:29

jmuehlner reviewed Feb 27, 2023

View reviewed changes

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from dc3f786 to 5458e6b Compare February 27, 2023 20:36

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from 5458e6b to d7331e3 Compare February 27, 2023 21:09

jmuehlner reviewed Feb 27, 2023

View reviewed changes

guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js Outdated Show resolved Hide resolved

GUACAMOLE-1740: Don't display clipboard contents in the clipboard edi…

2c15f3d

…tor until it is focused on.

aleitner force-pushed the KCM-186-Hide-Clipboard-Contents-By-Default branch from d7331e3 to 2c15f3d Compare February 27, 2023 23:25

jmuehlner approved these changes Feb 27, 2023

View reviewed changes

jmuehlner merged commit 4308bc1 into apache:master Feb 27, 2023

Conversation

aleitner commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmuehlner commented Feb 22, 2023

Uh oh!

jmuehlner Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmuehlner Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

jmuehlner Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

jmuehlner Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleitner commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmuehlner commented Feb 24, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmuehlner Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

jmuehlner Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aleitner commented Feb 22, 2023 •

edited

Loading

jmuehlner Feb 22, 2023 •

edited

Loading

aleitner commented Feb 24, 2023 •

edited

Loading