Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clicks aren't registering properly with Safari on xpra over HTML5 when using SSL #226

Closed
heinosasshallik opened this issue Feb 10, 2023 · 15 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@heinosasshallik
Copy link

heinosasshallik commented Feb 10, 2023

Describe the bug

When using Safari with XPRA served over HTML, over HTTPS, then when you left-click, the left click doesn't go through, and it says it shows a menu with "Paste" in it (as if you had right clicked).

Screenshot showing the menu with "Paste" in it that appears when left clicking:
2d95944a-9103-449b-9eca-424acdfef1ae

This issue only occurs when connecting to xpra over HTTPS, and not when connecting over HTTP. Keyboard keys still work. This issue does not occur with Chrome or Firefox.

To Reproduce
Steps to reproduce the behavior:

  1. Serve an application with xpra html5 mode
  2. Connect with safari over https
  3. Left click on anywhere in the application

I have created a minimal reproducible case for you to test this out.
minimal_test_case.zip

Steps to reproduce using the minimal reproducible case:

  1. Run docker-compose build to build the image
  2. Run docker-compose up to bring the image up
  3. On a mac machine with Safari, open the URL https://IP_ADDRESS_HERE:14500
  4. The password is xpra. Enter the password so you can see the application.
  5. Left click on the application

I served the application from a Linux machine and connected to it from a Mac, so for me, IP_ADDRESS_HERE was the IP address of the Linux machine on the local network.

System Information (please complete the following information):

  • Server OS: Ubuntu 20.04.5 LTS (Focal Fossa)
  • Client OS: MacOS Ventura 13.2
  • Xpra Server Version v4.4.3-r0
  • Safari Version: Replicated on 16.3 (18614.4.6.1.5) and 16.2 (18614.3.7.1.5)

Additional context
On Safari version 14.1.2 (16611.3.10.1.16), no image shows up at all, but that's probably a separate issue.

@heinosasshallik heinosasshallik added the bug Something isn't working label Feb 10, 2023
@totaam totaam transferred this issue from Xpra-org/xpra Feb 11, 2023
@totaam
Copy link
Collaborator

totaam commented Feb 11, 2023

This is probably a side effect of using the new clipboard API with Safari.
My guess is that Safari is trying to be smart and falling on its face.
The lazy and easy fix would be to not use the new clipboard API with Safari. The downside of that would be degraded clipboard functionality..
Commenting out these two lines should do that.

xpra-html5/html5/js/Client.js

Lines 2059 to 2060 in 953523e

$("#screen").on("click", (e) => this.may_set_clipboard());
$("#screen").keypress(() => this.may_set_clipboard());

@heinosasshallik does that work for you?

@heinosasshallik
Copy link
Author

No, it didn't work.

I don't have a lot of experience with safari developer tools, but I believe I successfully deleted those two lines using a response local override. Then I refreshed the page to apply the changes. The issue was still there.

screenshot

@totaam totaam added the help wanted Extra attention is needed label Feb 14, 2023
@totaam
Copy link
Collaborator

totaam commented Feb 14, 2023

I have no idea how to force Safari to accept my self signed certificate, all the examples I found online ended up just bringing me back to the "This connection is not private" certificate error page.
So I am unable to do anything about this issue.

@heinosasshallik
Copy link
Author

heinosasshallik commented Feb 14, 2023

@totaam You don't need to add a self signed certificate. Just go to "more info" and "visit the page anyway". You'll have an insecure connection, but the bug can be replicated like that easily.

Note: It's not actually called "more info" and "visit the page anyway", but it's something like that, and it exists in Chrome and Firefox too. You'll see the option when you're brought to the "This connection is not private" certificate error page.

Just in case, I'll mention again that I included an example docker image which will allow you to easily replicate the issue. Bring that up, go to http://IP_ADDRESS_HERE:14500, and accept the insecure connection. Or bring xpra up yourself, the docker image doesn't really do anything special. I'm just mentioning this in case you're doing something unnecessarily complex, like trying to use nginx as a reverse proxy to get SSL working like I tried in the beginning.

@totaam
Copy link
Collaborator

totaam commented Feb 14, 2023

You don't need to add a self signed certificate. Just go to "more info" and "visit the page anyway". You'll have an insecure connection, but the bug can be replicated like that easily.

No, that doesn't work here. That's the problem.

@heinosasshallik
Copy link
Author

Can you describe why or how it doesn't work? It works for me.

330899105_849545512785114_1391696838760579074_n

I would've made you a gif of me accepting the insecure connection, but it seems like Safari only complains about it once and if you accept it, it will not warn you again

@totaam
Copy link
Collaborator

totaam commented Feb 15, 2023

Can you describe why or how it doesn't work? It works for me.

Like I said above, it just goes in a loop back to the "This connection is not private" certificate error page.
I'll try to run a different MacOS VM / Safari version to see if that works any better.
But really, I'm getting frustrated with the amount of time I spend on MacOS dealing with issues that should be trivial to circomvent.

@heinosasshallik
Copy link
Author

heinosasshallik commented Feb 15, 2023

The frustration is understandable. I'd hate to deal with the certificate error problem as well.

In case it makes any difference, I'm using the latest versions of MacOS and Safari. If yours is not the latest version, then you can try upgrading both of them at once by choosing the "upgrade software" (or similar) option in your Mac's settings.

@totaam
Copy link
Collaborator

totaam commented Feb 15, 2023

then you can try upgrading both of them at once by choosing the "upgrade software" (or similar) option in your Mac's settings

I don't have any Apple hardware (that could run recent versions of MacOS) so I run MacOS virtualized and upgrading does not work...
Fortunately, it looks like my MacOS 12 VM (whatever the codename is) can reproduce the problem.
It is something to do with the clipboard, disabling it fixes this particular issue - so at least you have a workaround for that one.
But not the painting problem: #227

@heinosasshallik
Copy link
Author

Wonderful! Thank you so much!

When do you think we could get this fix into a new release? Just asking so I know whether to wait for the release or hot patch it in.

@totaam
Copy link
Collaborator

totaam commented Feb 15, 2023

I was going to make a new release this week, but MacOS has taken so much of my time that this looks unlikely now.

@heinosasshallik
Copy link
Author

heinosasshallik commented Feb 15, 2023

Replicating your workaround

I tried to replicate your workaround and it was a nightmare, but I think I got it working. No matter what I did, the bug would remain because the changes I tried to make weren't actually applying. I assume it's a caching issue.

I'll detail my steps in case anyone else tries to replicate this issue. In order to make sure the workaround was applying, I tried the following things:

  • Create a Response override with "skip network traffic" and verified in the network tab that the request to index.html was skipped. Made sure that "Disable caching" is enabled in the network tab.
  • Edited index.html in the docker container itself and restarted the service. I verified that the workaround is in place using curl.
  • Tunneled the xpra with the workaround to another port in an effort to bypass any caching logic on Safari's side
  • Made sure to write the workaround exactly as you wrote it (on the screenshot the const is replaced by var and there's no semicolon)

I verified that the workaround was in fact not in place by going to the elements tab, copying the HTML, pasting it into vim and seeing if the workaround code was there. Note: searching in the elements tab did not work properly in my version of Safari.

The only thing that finally worked was running the container on a completely different server and making sure the workaround is in place before ever connecting to xpra with Safari.

What a nightmare...

Conclusions

I can see that the workaround introduced this issue: #227, and that xpra is still unusable on Safari on HTTPS, even after the workaround. Like in your case, my window was also not drawn after I put the workaround in place.

I remember that your first suggestion was to comment out these lines:

xpra-html5/html5/js/Client.js

Lines 2059 to 2060 in 953523e

$("#screen").on("click", (e) => this.may_set_clipboard());
$("#screen").keypress(() => this.may_set_clipboard());

It's possible that the above solution would have also worked, and I just didn't see the changes due to the caching. I can try it out, if someone tells me how to get rid of Safari's caching without spinning up a new server each time.

@totaam
Copy link
Collaborator

totaam commented Feb 15, 2023

Commenting out those 2 lines was just my initial guess, but it turns out that any clipboard access triggers this Safari "feature" so the best thing we can do for now is to disable clipboard support when Safari is used with ssl connections.
That's what the commit above does.

Edit: the workaround has nothing to do with #227 - which is just another Safari "feature".

totaam added a commit that referenced this issue Mar 11, 2023
but we also want to change the default setting on the connect page
@totaam
Copy link
Collaborator

totaam commented Mar 16, 2023

There are so many things wrong with Safari, especially #227, that I'm not going to spend any time on this.
Disabling the clipboard is now the official solution for this Safari "feature".
Please use a better browser.

@totaam
Copy link
Collaborator

totaam commented May 14, 2024

Firefox is now broken in exactly the same way: #301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants