Support auto-redirection by `<link>` #106

saschanaz · 2023-03-26T22:34:14Z

Fixes #105

For now I didn't touch the existing detection mechanism. It could also detect more pages by some heuristic with regex on page URLs, but I'll defer that part.

saschanaz · 2023-03-26T23:14:37Z

Technically it could be faster with direct request redirection by reading HTTP headers, but:

That would require webRequestBlocking and would block Provide Chrome version #55
Misskey do not support headers but only HTML elements
Blocking all requests sounds very bad for general browser performance

saschanaz · 2023-03-27T19:37:17Z

BTW, I wonder what's the best to do when one wants to see the original page without redirection (because the fetched page in the home server may not show the full information). I'm deferring that too 👀

saschanaz · 2023-03-28T11:28:41Z

(This needs tests)

rugk · 2023-06-20T14:16:54Z

Hi @saschanaz,
first of all, thanks for your first contribution to this project! 🎉 👍 🏅
I hope you'll like this project and enjoy hacking on it… 😃

I am really sorry about the late reply, but I am quite busy with other life stuff and my GitHub backlog is growing exponentially.

Anyway, thanks a lot for that contribution (and idea BTW). I have created a milestone for that feature as it would change the UX of this add-on completely/drastically, but in a good way IMHO.

rugk

BTW I generally agree on keeping the old behavior as a fallback and for other instances etc.

rugk · 2023-06-20T14:21:49Z

src/_locales/en/messages.json

@@ -163,6 +163,10 @@
    "message": "Prevents the use of an extra tab and instead redirects the action on the main page.",
    "description": "This is an option shown in the add-on settings."
  },
+  "optionRedirectImmediately": {


Thanks a lot for introducing a setting!

IMHO, for a good UX we should then have two settings (both being enabled by default I guess):

one like this here enabling the "instant redirect" and

one covering the "old" redirects

This would enable one to disable the old redirect style (e.g. because of missing features/unwanted redirects like #28).

rugk · 2023-06-20T14:32:41Z

src/background/modules/ActivityPubRedirect.js

+        return;
+    }
+
+    await NetworkTools.redirectToWebsite(homeUrl, tabId);


Suggested change

await NetworkTools.redirectToWebsite(homeUrl, tabId);

return NetworkTools.redirectToWebsite(homeUrl, tabId);

Any special reason you use await instead of just returning the Promise? That could be helpful for error handling or so, that's why I'd prefer return if there is no reason.

Hmm? I believe awaiting is more helpful, since an error will throw at await call site but not at return call site. And awaiting is anyway more consistent.

hmm this goes deep into the way promises may be handled and I would have said before it makes no difference at all whether to await here, because likely where this is called it is also awaited. And await is just a cooler syntax for then.

But what you say may be right too, the stacktrace may not lost thus last function then? But I am really unsure here, so a js snippet/proof or some source would help. I am always open to learn something new hehe. Otherwise I'd stand to what I said, I.e. remove the await, because that would be consistent to how it is done in the code base everywhere else.

rugk · 2023-06-20T14:33:27Z

src/background/modules/AutoRemoteFollow.js

+    // clear cache of settings
+    await AddonSettings.loadOptions();
+    if (await AddonSettings.get("redirectImmediately")) {
+        redirectByActivityPubLink(tabId, changeInfo.url);


not awaiting this is deliberate?

Maybe put a commend to make that clear…

rugk · 2023-06-20T14:34:28Z

src/background/modules/AutoRemoteFollow.js

@@ -15,6 +15,7 @@ import * as MisskeyDetect from "./Detect/Misskey.js";

 import * as NetworkTools from "/common/modules/NetworkTools.js";
 import * as MastodonRedirect from "./MastodonRedirect.js";
+import { redirectByActivityPubLink } from "./ActivityPubRedirect.js";


BTW I usually prefer a "module-like style", i.e. import * and then do a ActivityPubRedirect.redirectByActivityPubLink() call that makes clear where the module is from.

rugk · 2023-06-20T14:35:42Z

src/common/modules/MastodonApi.js

@@ -99,3 +99,22 @@ export function getTootStatus(mastodonServer, localTootId) {
        return response.json();
    });
 }
+
+/**
+ * Resolves the ActivityPub object ID on a given server


any link/ref to the ActivityPub spec to add to this doc to exlain what it is? I've found one before, but I don't know whether that is up-to-date.

rugk · 2023-06-20T14:36:07Z

src/common/modules/data/DefaultSettings.js

@@ -6,5 +6,6 @@

 export const DEFAULT_SETTINGS = Object.freeze({
    ownMastodon: null,
-    redirectInMainWindow: false
+    redirectInMainWindow: false,
+    redirectImmediately: false,


Why not enabled by default?

rugk · 2023-06-20T14:49:48Z

I am answering the questions here now here:

Use the injection code to add "open it in my home server" button (perhaps with a home icon) for some supported server implementations (so that one can see some extra info in the original page)

If not, add a position: absolute fallback popup with the same button on the top of the page

I can imagine both being useful. But a popup may actually be the coolest thing – maybe in the design of that "copy that link" popup Mastodon shows by default, so that would be a replacement for it and integrate more seemingly.

However, I have another idea:

What about saving the lastest redirected URL/toot/user, and – if that URL is opened again only then show that popup aka „Do you really want to redirect?“. In other words: Only redirect if the toot has not been recently redirected already.
This would save the click in 80% of the cases while keeping the ability to see the original toot when you have a reason for that.

If you wanted a quick implementation/MVP of it, you could just treat it as „no i never want to redirect if the URL is opened again” and not implement the popup for now. Then you could test how it feels like when using and maybe we don't actually need a popup?

I could imagine this would be a quite intuitive way, or otherwise one cannot differentiate that that may be an error? I don't know whether and how we could communicate that, if so, but we could always make it an optional feature to be enabled in the settings.

saschanaz added 2 commits March 27, 2023 00:31

Support auto-redirection by <link>

c14eef0

tabId

255a9ca

saschanaz mentioned this pull request Mar 27, 2023

Standardize discovery using link rel on user-visible URLs w3c/activitypub#310

Open

saschanaz marked this pull request as draft March 28, 2023 11:28

rugk added this to the v3.0 milestone Jun 20, 2023

rugk requested changes Jun 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support auto-redirection by `<link>` #106

Support auto-redirection by `<link>` #106

saschanaz commented Mar 26, 2023

saschanaz commented Mar 26, 2023

saschanaz commented Mar 27, 2023

saschanaz commented Mar 28, 2023

rugk commented Jun 20, 2023

rugk left a comment

rugk Jun 20, 2023

rugk Jun 20, 2023

saschanaz Aug 5, 2023

rugk Aug 7, 2023

rugk Jun 20, 2023

rugk Jun 20, 2023

rugk Jun 20, 2023 •

edited

Loading

rugk Jun 20, 2023

rugk commented Jun 20, 2023 •

edited

Loading

	await NetworkTools.redirectToWebsite(homeUrl, tabId);
	return NetworkTools.redirectToWebsite(homeUrl, tabId);

Support auto-redirection by <link> #106

Are you sure you want to change the base?

Support auto-redirection by <link> #106

Conversation

saschanaz commented Mar 26, 2023

saschanaz commented Mar 26, 2023

saschanaz commented Mar 27, 2023

saschanaz commented Mar 28, 2023

rugk commented Jun 20, 2023

rugk left a comment

Choose a reason for hiding this comment

rugk Jun 20, 2023

Choose a reason for hiding this comment

rugk Jun 20, 2023

Choose a reason for hiding this comment

saschanaz Aug 5, 2023

Choose a reason for hiding this comment

rugk Aug 7, 2023

Choose a reason for hiding this comment

rugk Jun 20, 2023

Choose a reason for hiding this comment

rugk Jun 20, 2023

Choose a reason for hiding this comment

rugk Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

rugk Jun 20, 2023

Choose a reason for hiding this comment

rugk commented Jun 20, 2023 • edited Loading

Support auto-redirection by `<link>` #106

Support auto-redirection by `<link>` #106

rugk Jun 20, 2023 •

edited

Loading

rugk commented Jun 20, 2023 •

edited

Loading