[WIP] Possible Archive.org check implementation #176
Conversation
src/js/core/background.js
Outdated
| * | ||
| * @type {boolean} | ||
| */ | ||
| disableAutomaticArchiveOrgReplacement: false, |
There was a problem hiding this comment.
Not implemented yet, also no check for this setting.
| @@ -540,6 +547,60 @@ const bookmarksorganizer = { | |||
| } | |||
| }, | |||
|
|
|||
| /** | |||
| * This method sends a GET request to the archive.org API to check if there is a snapshot available for a given URL. | |||
| * It replaces the current broken URL with the archived URL. | |||
There was a problem hiding this comment.
That is currently not true, as I imagined it to be more clean to use it as a NewUrl and take the redirect approach.
|
|
||
| const archived_snapshots = await response.json(); | ||
|
|
||
| console.log(archived_snapshots); |
| else { | ||
| bookmark.newUrl = `${snapshot_base_url}${bookmark.url}`; | ||
| bookmark.status = STATUS.REDIRECT; | ||
| } |
There was a problem hiding this comment.
maybe as an addition here, we could rename the bookmark. Though I'm not sure this is working out well with the approach to threat it as a possible redirect.
There was a problem hiding this comment.
Hi,
thank you very much for your contribution! Do you know an example URL that can be used for testing?
I am not sure if it's the right approach to mark a bookmark as redirected if there is a snapshot saved on archive.org. I would argue that the user should be notified about the snapshot but not in the same way as with a redirect because the original bookmark would still be kind of broken.
The UI of the Bookmarks Organzier knows errors (red dot; broken bookmarks) and warnings (yellow dot; so far only redirects). I think a warning is okay if there is a saved snapshot but instead of showing the old URL and the new URL it should be shown a link to archive.org. For this we probably need a new status code in status.js and work with that in any way. Similar to the link to optionally correct the URL for a redirect we would need a link to optionally change the URL to the archive.org link. It also has to be checked that the feature to automatically repair all redirects does not change the URL of such a bookmark (because it's not a redirect) and as such shouldn't be affected by this batch operation.
| @@ -612,7 +673,11 @@ const bookmarksorganizer = { | |||
| }); | |||
| } | |||
|
|
|||
| if (bookmark.status > STATUS.REDIRECT) { | |||
| if (bookmark.status == STATUS.NOT_FOUND) { | |||
There was a problem hiding this comment.
Please always use strict equality operators (=== instead of ==).
By the way: If you run npm run lint:js you can run the linter. It complains about things like that. :)
|
closed due to inactivity. |
Hey, yes sorry. Lost a bit track of it due to other projects. |
|
Not a problem at all! Just let me know if you want to give it a try again. :) |
I'm unsure, as I'm more working in Systems Programming (Rust) and not really know much about TS/JS. So I'm probably not the best for the job 😅 as I don't know all the patterns usually being used. I guess it would be much easier for someone actually used to the language to implement it in a good way. Thing is, I can't use |
Started sketching out a possible implementation. This is WIP though, wanted to PR early for feedback and directions.
For now I relied on the
redirectimplementation and just checked for a404in a bookmark, if so check with archive.org. If there is no snapshot available keep theNOT_FOUNDelse return thebookmarkwith aNewUrland set status toREDIRECT.What do you think @cadeyrn?
When ready:
Fixes #147