GoogleChromeLabs / link-to-text-fragment

Browser extension that allows for linking to arbitrary text fragments.
https://chrome.google.com/webstore/detail/link-to-text-fragment/pbcodcjpfjdpcineamnnmbkkmkdpajjg
Apache License 2.0
412 stars 33 forks source link

Does not work on Safari for MacOS and is not marked as compatible with iPad #69

Open CollinsVakayil opened 2 years ago

CollinsVakayil commented 2 years ago

I'm on macOS Monterey Version 12.4 (M1 Pro chip) MacBook Pro.

tomayac commented 2 years ago

I can reproduce, but have no idea what has changed. It was working fine at the time the extension was released. I need to investigate, but can’t really commit to a time this will happen. If you have cycles, happy for your help!

CollinsVakayil commented 2 years ago

I can reproduce, but have no idea what has changed. It was working fine at the time the extension was released. I need to investigate, but can’t really commit to a time this will happen. If you have cycles, happy for your help!

I'm happy to help any way I can, unfortunately I don't know how to code. So forgive my ignorance on terminology when I ask what are cycles and how do get them to you.

tomayac commented 2 years ago

Oh, sorry. In the world of computer science cycles refers to CPU cycles (that is, the time required for a simple processing operation). The term is colloquially also used to refer to time a developer can dedicate to do something.

ahmetasabanci commented 2 years ago

Also having the same issue.

@tomayac If there's anything I can do to help you investigate, just let me know.

tomayac commented 2 years ago

PSA for people needing a solution now: Safari Technology Preview 148 lets you toggle an experimental flag that enables the feature:

Screenshot 2022-07-15 at 16 40 38

(Edit: So existing links work, this is not for creating links.)

RokeJulianLockhart commented 2 years ago

@tomayac, do you know how to automate enablement of that feature via the command-line, or how I might ascertain how to?

tomayac commented 2 years ago

Changed the title to reflect the related problem reported in the closed #62.

tomayac commented 2 years ago

@tomayac, do you know how to automate enablement of that feature via the command-line, or how I might ascertain how to?

I don't know, sorry.

jasmas commented 1 year ago

I get an error in the javascript console in Safari that reads: Error: Unchecked runtime.lastError: This extension already contains a context menu item with identifier 'copy-link'.

Not sure if that's related or helpful

JayHoltslander commented 1 year ago

I was so excited for this upon discovering the existence of text fragments this morning. Sadly the Safari version of the plugin is not working for me on my 5 year old Mac running Mac OS 11.7.1 (The highest it can go.) Clipboard is not populated with a link and instead remains full of previously copied text. I'm sad because in my day to day surfing I prefer Safari over Chrome.

RokeJulianLockhart commented 1 year ago

@JayHoltslander, have you tried Linux on your Mac? I don't think that asking for support for an unsupported build of Safari on a 5-year-old Mac is very reasonable.

JayHoltslander commented 1 year ago

@rokejulianlockhart This is my employer provided machine. I don’t think a 5-year-old Mac should be considered ancient or unreasonable to hope is supported. 🤷🏻‍♂️

RokeJulianLockhart commented 1 year ago

Indeed, that sort of age for most computers isn't problematic, but you're running an unsupported OS. That universally is. It's not even an LTS, right, @JayHoltslander?

jdelStrother commented 1 year ago

I poked at this for a bit, though without much success.

The first problem is that injectContentScripts pings the tab to see if the content script has been injected, but browser.tabs.sendMessage never completes - it just seems to behave like a black hole.

I can force the injection with something like this -

--- a/LinkToText Extension/Resources/background_script.js
+++ b/LinkToText Extension/Resources/background_script.js

@@ -26,26 +26,33 @@
       console.log(...args);
     }
   };

+let firstTime = true;
   const injectContentScripts = async (contentScriptNames) => {
     // If there's a reply, the content script already was injected.
     try {
+        if (firstTime) {
+          firstTime = false
+          throw new Error('force injection')
+        }
      return await sendMessageToPage('ping');
     } catch (err) {

after which sendMessageToPage behaves as expected, and the content script generates a linked url.

Unfortunately, even then copying doesn't work. It tries to use the fallback copy method but nothing happens (maybe because Safari will only copy-to-clipboard on a user event and it's not seeing this as a user event?).

Fingers crossed that maybe Safari 17 will just add native support.... 😕

tomayac commented 1 year ago

It’s surprisingly frustrating, especially since it worked fine when I released it initially on the App Store. I personally can’t justify the time investment at the moment. Thanks for investigating, though!