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

Does not work on local web pages in htmlz format #8

Closed
kramola-RU opened this issue Jul 25, 2023 · 14 comments · Fixed by #9
Closed

Does not work on local web pages in htmlz format #8

kramola-RU opened this issue Jul 25, 2023 · 14 comments · Fixed by #9
Labels
bug Something isn't working

Comments

@kramola-RU
Copy link

Hi!
The addon does not work on local web pages saved with SingleFile (SingleFileZ).
Could you add the ability to work on saved local files?

@farblos
Copy link
Owner

farblos commented Jul 25, 2023

I could reproduce your issue (not working on files saved with SingleFileZ) for the first time after I installed SingleFileZ.

After restarting the browser and opening a *.zip.html page copy-on-select somehow started working even on that previously saved page.

Will try again tomorrow with a fresh browser profile - if you have some more detailed instructions on how to reproduce the issue, pls let me know.

@kramola-RU
Copy link
Author

if you have some more detailed instructions on how to reproduce the issue, pls let me know.

I open any file saved in SingleFileZ and the addon does not work.
I'm using Firefox 117a1

For example - this file:
https://www.upload.ee/files/15495391/example.7z.html

@farblos
Copy link
Owner

farblos commented Jul 26, 2023

Thanks for the sample - I can reproduce the issue now. Will have a closer look tomorrow.

@farblos farblos changed the title Does not work on local web pages Does not work on local web pages in htmlz format Jul 27, 2023
@farblos
Copy link
Owner

farblos commented Jul 27, 2023

The good news: There is hope.

The bad news: It's a timing related issue - if I sleep 50ms before adding the main event listener things seem to work. Not beautiful.

Since you are on FF nightly and a GitHub user, would you mind testing an unsigned extension that provides this fix? I have pushed the tentative changes to branch "8-does-not-work-on-local-web-pages-in-htmlz-format". If you clone that branch you should be able to use that clone as temporary extension. Alternatively, I can provide an unsigned xpi.

If you need instructions on all that, just let me know. If that's all too much, also fine - only having such a hack asks for more tests, I think.

Thanks.

@kramola-RU
Copy link
Author

I have pushed the tentative changes to branch "8-does-not-work-on-local-web-pages-in-htmlz-format".

Thank you very much!
This version works fine with .htmlz files.

@farblos
Copy link
Owner

farblos commented Jul 27, 2023

Now that was quick - thanks as well for testing!

Will need some time to finalize the changes and test some more - stay tuned...

@farblos
Copy link
Owner

farblos commented Jul 29, 2023

Found the root cause of the issue, nothing funny or strange about this. The body of the document changes after it has been loaded, thus loosing the initially attached event listener. This isn't particularly difficult to fix, I just haven't had this on my radar before. In any case this will require a different/better fix.

@kramola-RU
Copy link
Author

The body of the document changes after it has been loaded, thus loosing the initially attached event listener.

But some addons are not affected by this problem.
For example, Gesturefy and Drag-Select Link Text.

@farblos
Copy link
Owner

farblos commented Jul 29, 2023

But some addons are not affected by this problem.
For example, Gesturefy and Drag-Select Link Text.

Most likely because they did this thing correctly right from the start :-)

After all, I started this fork because I needed this functionality myself and found the original extension a bit too limited. Now I keep learning more and more JavaScript with every bug a user comes across ...

So if you find any such issues in future, please do file them - very much appreciated!

I hope to have a final version fixing this issue ready by end of this weekend.

@kramola-RU
Copy link
Author

I hope to have a final version fixing this issue ready by end of this weekend.

Super! I'll be happy to test that version.

@farblos
Copy link
Owner

farblos commented Jul 30, 2023

Just released version 2.5. Please give it a try, when all is good I'll upload it to addons.mozilla.org.

@kramola-RU
Copy link
Author

give it a try

2.5 works great. Brilliant! Thank you very much!
I have one question:
I want to disable restrictions for all websites.
Will it be enough to just remove these lines from the script?

// ignore events on some sites
if ( (new URL( document.URL )).hostname === "docs.google.com" )
  return;

let s = document.getSelection().toString();
let t = e.target;

Or do I need to remove some additional lines?

@farblos
Copy link
Owner

farblos commented Jul 30, 2023

2.5 works great.

Thanks for testing, will upload to AMO soon.

Will it be enough to just remove these lines from the script?

You should only remove these three lines:

// ignore events on some sites
if ( (new URL( document.URL )).hostname === "docs.google.com" )
  return;

but leave the others in - the let statements are important for the rest and have nothing to do with the site restriction.

This restriction, by the way, comes from issue #1. The problem is that on docs.google.com this extension breaks regular copy and paste.

Closing this issue - if you have more questions, feel free to continue in the closed issue or open another one ...

@farblos farblos closed this as completed Jul 30, 2023
@farblos farblos linked a pull request Jul 30, 2023 that will close this issue
@farblos farblos added the bug Something isn't working label Jul 30, 2023
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 a pull request may close this issue.

2 participants