MrOtherGuy / bookmark-batch-updater

A webextension tool to update multiple bookmarks at once
Mozilla Public License 2.0
10 stars 1 forks source link

unable to replace the string about:reader?url= #6

Closed ghost closed 3 years ago

ghost commented 3 years ago

I'm attempting to replace (with nothing) the about:reader?url= strings which is added by the Firefox reader mode. The reader URLs always start with the string, the regex I use is (70 matches reported by the addon):

about:reader\?url\=

results in the error:

Failed operations matching about:reader\?url\= 70

function match() { [native code] }


Firefox 84.0.2 (64-bit) for Fedora

bookmark-batch-updater v0.9

MrOtherGuy commented 3 years ago

What are you using as the replace value in that case? And could you share some urls that fail?

I'm not sure where you get function match() { [native code] } although one possible cause is fixed in my v1.0 dev build - but nonetheless you will get a failure if the resulting url is not a valid url.

Well, the heuristic on "what is a valid url" is indeed pretty stupid but it is there to filter out cases that are very likely not valid such as if the result ends with having host-name that starts or ends with . or includes .. all of which could result in badly crafted replacement value.

ghost commented 3 years ago

What are you using as the replace value in that case? And could you share some urls that fail?

A blank line, but I also tried with random characters. Removing the about:reader?url= would restore the standard URL and there's no need to use anything as the replacement value.

about:reader?url=http%3A%2F%2Fwww.rogerebert.com%2Frogers-journal%2Fthe-shaky-queasy-utimatum
about:reader?url=http%3A%2F%2Frome.ro%2Fquakes-player-speed-1
about:reader?url=https%3A%2F%2Fwww.eurogamer.net%2Farticles%2F2018-07-04-wreckfest-review-a-true-successor-to-the-brilliant-destruction-derby
about:reader?url=https%3A%2F%2Fwww.videogamer.com%2Ffeatures%2Ftimesplitters-creator-interview-goldeneye-future-perfect-and-the-koch-media-acquisition  
about:reader?url=https%3A%2F%2Fwww.rockpapershotgun.com%2F2011%2F07%2F04%2Fwot-i-think-alpha-polaris%2F
about:reader?url=https%3A%2F%2Fadventuregamers.com%2Farticles%2Fview%2F29419
about:reader?url=https%3A%2F%2Fnoisey.vice.com%2Fen_us%2Farticle%2Fj5gn9k%2Frank-your-records-mastodon-brann-dailor

The copied URLs are in the encoded form -- this is how they are displayed in the Firefox bookmarks, if that matters:

about:reader?url=http://www.rogerebert.com/rogers-journal/the-shaky-queasy-utimatum
about:reader?url=http://rome.ro/quakes-player-speed-1
about:reader?url=https://www.eurogamer.net/articles/2018-07-04-wreckfest-review-a-true-successor-to-the-brilliant-destruction-derby
about:reader?url=https://www.videogamer.com/features/timesplitters-creator-interview-goldeneye-future-perfect-and-the-koch-media-acquisition  
about:reader?url=https://www.rockpapershotgun.com/2011/07/04/wot-i-think-alpha-polaris/
about:reader?url=https://adventuregamers.com/articles/view/29419
about:reader?url=https://noisey.vice.com/en_us/article/j5gn9k/rank-your-records-mastodon-brann-dailor
ghost commented 3 years ago

I tried once again with placeholder string:

Screenshot from 2021-01-30 12-23-09

MrOtherGuy commented 3 years ago

Thank you. You are actually seeing two separate problems, one of which should be fixed already in v1.0. The other is that the extension does not handle those encoded forms at all. But that should be rather simple to fix.

MrOtherGuy commented 3 years ago

Actually on second thought this could be somewhat tricky. What makes it tricky is that since you are now removing the about:reader part then the resulting url really is not valid anymore - I mean I can make the extension handle that, but Firefox does not want to store a bookmark that begins with http%3A%2F%2 because the url really must be in non-encoded form.

The original about:reader url only works because the query string in the url can be in encoded form but the protocol+domain+path cannot be.

In principle I could make the extension figure out a valid form, but I'm a bit vary because that would mean that it will do modifications to bookmarks that the user did not ask to do.

ghost commented 3 years ago

Thank you for the information. I'll consider handling the bookmarks somewhat manually and disabling the reader mode (which I never found particularly useful since it fails to parse some elements and never worked with paginated articles) by setting reader.parse-on-load.enabled to false.

Perhaps you could create a new category/tab for the about:reader, next to the existing three, where this new hypothetical "reader mode logic" would only be used.

MrOtherGuy commented 3 years ago

It's not really just affecting about:reader but any bookmark that would be of form <url>?<encodedUrl> where the intention is to remove the ? and use a non-encoded form of

Still, as a workaround I added a checkbox that, when enabled will make the extension attempt to fix the urls during the update rather than just failing. I think that's a good compromise.

MrOtherGuy commented 3 years ago

This should be fixed by cf1d1a593919f87df9108ed4ea32519f0e1b556b

MrOtherGuy commented 3 years ago

Hi, I have published an unsigned version of the v1.0 here

It would be great if you could test if it works for your purposes before I publish it in to the addons store. I can't find any obvious flaws anymore, but evidently there can be flaws that I just couldn't think of.

The package is unsigned, so you can only install it in release Firefox by going to about:debugging and from there use the "install temporary extension" feature.

Thanks!

ghost commented 3 years ago

Hi, I have published an unsigned version of the v1.0 here

It would be great if you could test if it works for your purposes before I publish it in to the addons store. I can't find any obvious flaws anymore, but evidently there can be flaws that I just couldn't think of.

The package is unsigned, so you can only install it in release Firefox by going to about:debugging and from there use the "install temporary extension" feature.

Works fine with the 'try fix URLs' option enabled, I manually verified at least the examples mentioned above.

Update success:70 bookmarks were updated

One downside I noticed is that this process can create exact duplicates, albeit I'm not sure if Firefox allows them to exist in the same folder (active users of the reader mode likely have bookmarked many articles twice). You could consider adding an option to clear the dupes out as well.


Also, my router IP was now excluded automatically.

MrOtherGuy commented 3 years ago

Very cool, thanks a lot! I'll push the update to the addons store later today.

You can open a new issue about what to do with duplicates if you want, but that is nothing new, you always could have done that. Firefox doesn't care if there are duplicates and it's not even inherently "wrong" to have duplicates. So such a feature is not going to be in v1.0 and really I'm not even sure if I want to add it to the extension in the first place.

ghost commented 3 years ago

@MrOtherGuy I found a new reader mode string moz-extension:// from my bookmarks (likely originates from mobile, Firefox Fenix):

moz-extension://dd328262-078e-4b4a-ad8a-a2b9ce955dbc/readerview.html?url=https%3A%2F%2Fwww.hollywoodreporter.com%2Fnews%2Fbiopic-motorhead-frontman-lemmy-kilmister-works-1298635
moz-extension://d9a6ca70-ca40-45b0-96ed-0cf11fb3fcac/readerview.html?url=https%3A%2F%2Fclonezilla.org%2Fshow-live-doc-content.php%3Ftopic%3Dclonezilla-live%2Fdoc%2F03_Disk_to_disk_clone&id=137438955930

decoded:

moz-extension://dd328262-078e-4b4a-ad8a-a2b9ce955dbc/readerview.html?url=https://www.hollywoodreporter.com/news/biopic-motorhead-frontman-lemmy-kilmister-works-1298635
moz-extension://d9a6ca70-ca40-45b0-96ed-0cf11fb3fcac/readerview.html?url=https://clonezilla.org/show-live-doc-content.php?topic=clonezilla-live/doc/03_Disk_to_disk_clone&id=137438955930

Neither opens on desktop, I manually copied the URLs after ?url=.

...if you want to add any clearing options for the about:reader URLs.