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

Fix right-frame google translation link. #59

Merged

Conversation

jzohrab
Copy link

@jzohrab jzohrab commented Nov 4, 2022

When using a Google Translate URL, every "translate click" fails on the first click, which is annoying. This is a very simple fix.

The current pre-PR code uses a series of calls to eventually open a new window with a header Location change. I believe that Google has blocked this for XSS attack reasons. This change just makes the proper URL using plain javascript and then opens the window. It's very clear what's happening with this new code, but I don't know enough project history to say if this is an acceptable change or not.

Note that the 'translate sentence' in the reading pane pop-up is still broken, because it uses a slightly different set of calls, and still relies on the Location header change.

Issuing this PR to master because master is currently broken, so it's a hotfix.

The current code uses a series of calls to eventually open a new window with a header
Location change.  I believe that Google has blocked this for XSS attack reasons.
This change just makes the proper URL using plain javascript and then opens the window.
It's very clear what's happening with this new code, but I don't know enough project
history to say if this is an acceptable change or not.

Note that the 'translate sentence' in the reading pane pop-up is still broken,
because it uses a slightly different set of calls, and still relies on the Location
header change.
@jzohrab
Copy link
Author

jzohrab commented Nov 4, 2022

Fixes #58 -- but apparently I should edit a different file, as this file is the minified file. Need some guidance from @HugoFara as it's not clear to me which file to edit, or how to run the minifying process (that should be documented for future devs as well).

@jzohrab
Copy link
Author

jzohrab commented Nov 5, 2022

PR updated, js code moved to correct spot and minify run. Whoops! Should have read docs/contribute :-/

@HugoFara HugoFara added the bug Something isn't working label Nov 7, 2022
@HugoFara HugoFara merged commit b388cf2 into HugoFara:master Nov 8, 2022
@HugoFara
Copy link
Owner

HugoFara commented Nov 8, 2022

Hello! I've merged your pull request after some adjustments!

Finally the solution I've opted for was to rewrite the createTheDictLink JS function. It was simply calling PHP function createDictLink before, now it does the same thing as before but in pure JS. I had to do so to consider the case when there is no '###' in the dict URL, so you have to append content to the end of the string.

I removed the case where two '###' where present in the URL, because it was indicating a non-UTF-8 encoding. Back in 2011 (LWT 1.0.2), it may have been a good idea but it does no seem of any use today, and made the code unreadable.

@jzohrab jzohrab deleted the iss_58_fix_google_translate_sentence branch November 15, 2022 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google translate fails for whole sentence, but succeeds for a phrase with the exact same terms.
2 participants