search

CanisLupus / swift-selection-search

Swift Selection Search (SSS) is a simple Firefox add-on that lets you quickly search for some text in a page using your favorite search engines.
https://addons.mozilla.org/firefox/addon/swift-selection-search/
MIT License
213 stars 27 forks source link

Trying to select certain text within any RDF page causes the page to go cray cray ;) #245

Closed Gitoffthelawn closed 2 years ago

Gitoffthelawn commented 2 years ago

Hi Daniel! I hope you are doing well. I came across an issue and thought perhaps it's significant enough to report.

Describe the problem

When SSS is enabled, trying to select certain text within any RDF page (or file) causes the page to go cray cray. ;)

How to reproduce the problem

Steps to reproduce the behavior:

  1. Go to https://download.imagemagick.org/ImageMagick/download/binaries/digest.rdf
  2. Select any of the text within any one of the SHA256 hashes you see.
  3. Watch the page go cray cray. The entire page will lose it's formatting, and what you selected will no longer be selected.

System info

CanisLupus commented 2 years ago

Hi! Oh wow, this is very interesting (and quite bad) 😅 Thanks for reporting it.

It seems to also happen with other kinds of XML files. Even though they are not HTML webpages, Firefox tries to display them in a formatted way (which is useful). The problem is that SSS doesn't seem to care that this is not HTML and still tries to create the popup in the (not-)webpage:

image

It doesn't manage to create much beside an empty skeleton of an element, but that's enough for Firefox to not like it and completely break the display of the XML, which, to be honest, is completely fair.

I could take a look when possible. Not sure how to approach the fix, but it definitely shouldn't do this! I'm already surprised that this is a page SSS can (and does) register with.

Cheers!

CanisLupus commented 2 years ago

And yes, I'm doing well here, thanks! :) I hope you are too!

Gitoffthelawn commented 2 years ago

And yes, I'm doing well here, thanks! :) I hope you are too!

Good to hear! I'm doing well too. I'm too busy for my taste, but I'm trying to simplify things so there's less on my plate and more time for fun.

Regarding the RDF/XML file issue, I'm glad to hear it wasn't just a mistake I was making or something like that.

It's been a long time (years) since I've looked closely at the SSS code, but maybe you can require the presence of the opening <HTML tag for web pages. Either that, or maybe look for <!DOCTYPE html>.

A more hacky technique would be to change behaviour dependent on the file extension of the rendered page. Of course, that's not as reliable as one would want.

A super-duper incredibly hacky method would be to try something like enveloping <sss-popup> inside <digest:Content></digest> and hope it doesn't mess up regular web pages. Now, there's an ugly idea that I hope doesn't become a reality! :)

I'm sure you'll think of something good... you always do.

Cheers to you and yours. I hope we stay connected over the years.

CanisLupus commented 2 years ago

Good to hear! I'm doing well too. I'm too busy for my taste, but I'm trying to simplify things so there's less on my plate and more time for fun.

Nice! Good to hear as well. Don't overwork yourself if you can! :)

Regarding the RDF/XML file issue (...)

Good suggestions! The first one is along the lines of what I was thinking. The second one will likely depend on lots of edge cases (ex.: a URL without the "file" extension) and the last one is... yeah, I agree with you on that one; that's probably not gonna happen hahah.

I'm sure you'll think of something good... you always do.

❤

Cheers to you and yours. I hope we stay connected over the years.

Likewise! Cheers, and I'll let you know when I have more info on this (I still haven't checked).

CanisLupus commented 2 years ago

Hey hey hey! SSS v3.47.5 has a fix for this. The fix is really simple, as it just checks for the presence of the html tag as the documentElement. Note: in the previously failing page, the root is rdf:RDF instead of html.

image

I researched a few options before and this seemed like the best one. Using doctype could fail for old/odd webpages that simply don't define it, but the HTML root always be there, even if the browser is just displaying a file from your computer as text. XML is different since the browser has a special view for it.

Now let's hope there are no weird websites where this fails for some outrageous reason. 😂

When the version is published and you have some time, please confirm that you also see this fixed on your end. :)

Cheers and thanks again for the report! Daniel

Gitoffthelawn commented 2 years ago

Hey hey hey! SSS v3.47.5 has a fix for this. ... Cheers and thanks again for the report! Daniel

Hey hey hey! That looks great! Nice and simple. Thanks so much. And, of course, you're most welcome.

Since SSS is a recommended extension, Mozilla should make the new version available on AMO in the next day or two. I'll test the new release within a few hours of it being available on AMO.

The change you made could actually, possibly, even improve Firefox performance just a touch because it's possible that documentElement is not set to html until real HTML pages start getting received from the servers. I haven't tested that hypothesis, so it's just a thought... but it's a happy thought. :)

CanisLupus commented 2 years ago

You're welcome! :)

😄 In this case SSS was already relying on documentElement existing by the time the script is activated on the page (which is after browser.webNavigation.onDOMContentLoaded), so sadly there won't be improvements there. But well, no more broken XMLs 😁

Gitoffthelawn commented 2 years ago

Ah! Makes good sense. Thanks. And, of course, you're most welcome.

I tested the new release, and your fix took care of the issue wonderfully. I haven't noticed any problems at all with your patch.

Thanks for such a quick fix! Be well... hopefully talk with you soon.

CanisLupus commented 2 years ago

:) Awesome, glad to hear it! I'm going to close this but, of course, if you find something wrong feel free to reopen it.

It was no problem. Let's get these bugs fixed :) Hope so too! ❤ Cheers!