Use hierarchy of fallback selectors for injection

[vaijns]

To get mobile working as requested in #10 I've decided to not go the route of having mobile-specific code but the possibility to have a viable list of selectors for each search engine, with optional transformations before (and after failed query for the element).
Also, instead of just checking if the element is null, I also check if it is visible or not.

This maps nicely with the special-case Google had already.

What it also allows, is adding mobile support on-demand, when someone has the time to figure out how to do it for a certain search engine, as I'm not sure if I'll find the time to do it for all. Also might not find the time for looking at styling issues very much right now, as it doesn't look that pretty on mobile.

However, it can serve as a baseline, with some form of mobile "support" already (it's visible without desktop mode at least), which is a good first step I think.

Please let me know, what you think of this approach.

[vaijns]

After the latest commit with a small change for Google, I think it actually doesn't look bad at all.

If the images for mobile seem stretched, open them in a new tab and they should look normal again.

Using Duckduckgo on desktop (should be the same as before):

Using Google on desktop (should be the same as before):

Using Google on desktop if the sidebar wasn't there (should normally never happen, I forced it by temporarily removing the selector which creates the sidebar):

Using Google on mobile in desktop mode if the sidebar wasn't there (should normally never happen, I forced it by temporarily removing the selector which creates the sidebar):

Using Google on mobile in desktop mode:

Using Google on mobile:

Using Duckduckgo on mobile in desktop mode:

Using Duckduckgo on mobile:

[vaijns]

Oh, forgot showing Duckduckgo with no sidebar on desktop, which again should never happen:

I haven't tried the same thing on mobile in desktop mode but you get the idea, it doesn't seem to cause any problems.

Oh, and you might wonder how this looks if there's some AI stuff in either of those, in case you don't have it disabled:

As you can see, for Duckduckgo it is above the AI stuff, for Google it is below.

Some other things (like ads or definitions for words) seem to appear below the injection for Google. I haven't specifically tried this with mobile but It's likely gonna be similar.

Might be a good idea to try making this more consistent in the future but I don't think it would be a priority for this.

[Fivefold]

Hey @vaijns, thank you so much for taking the time to tackle this! This is a really clean PR, I like it a lot. I also appreciate the screenshots.

Your approach looks reasonable, however I wonder how easy it is to actually use different css or even content for mobile devices. I could imagine it would be nice to have the injector box collapsible and be collapsed by default, as it can get quite large vertically and a user will not always be interested in the linkding results. In the sidebar this is not an issue, as the normal results are right next to it, on mobile however the linkding box should not take up the whole screen by default in my opinion. I'm not sure this could be done in a pure css fashion. If not it will require mobile-specific code again.

Still, this is something that could be added later, and nobody is forced to use the mobile extension.

Before merging this it definitely needs to be checked for all supported search engines though. I know this is a lot of work, so you don't need to do it if you don't want. Unfortunately, I'm very busy over the next months, so it could be a while before I can handle it myself. If you do check the other search engines, more screenshots would be appreciated!

Also, can you add some instructions on how to run the debug/dev build on a mobile device? I do not have experience with this, and evidently you already figured it out.

[vaijns]

I'd say the essential goal in this PR is to not make it about being on mobile or not but rather about which position we inject into.

So in a similar fashion, I think there could be another property for a CSS class that is applied when a certain selector is used.
Based on this specific styling could be applied. And if there still should be something different for mobile then, it should probably be a job for CSS media queries or container queries.
Or can you think of something that likely couldn't be handled with such an approach?

Debugging turned out easier than I thought, with help of this article. This had everything I needed. If something seems unclear, I can certainly try to help, although I'm no expert regarding this either. :D

Basically it was unzipping the artifact after the build and then using the web-ext run -t firefox-android --adb-device XXX --firefox-apk org.mozilla.firefox command mentioned in the article in the unzipped directory.
For quick changes I just did a cp src/searchInjection.js path/to/artifact/build/searchInjection.js and then pressed R in the terminal where I've started the web-ext run to reload.

As for testing the other search engines, I'll see what I can do!

[Fivefold]

So in a similar fashion, I think there could be another property for a CSS class that is applied when a certain selector is used. Based on this specific styling could be applied. And if there still should be something different for mobile then, it should probably be a job for CSS media queries or container queries. Or can you think of something that likely couldn't be handled with such an approach?

It's probably fine, I can't think of a specific example where I'm sure it would not work. And even if there is, there's always javascript as the heavy-duty workaround.

Debugging turned out easier than I thought, with help of this article. This had everything I needed. If something seems unclear, I can certainly try to help, although I'm no expert regarding this either. :D

Basically it was unzipping the artifact after the build and then using the web-ext run -t firefox-android --adb-device XXX --firefox-apk org.mozilla.firefox command mentioned in the article in the unzipped directory. For quick changes I just did a cp src/searchInjection.js path/to/artifact/build/searchInjection.js and then pressed R in the terminal where I've started the web-ext run to reload.

Thank you, this will come in handy in the future for sure!

As for testing the other search engines, I'll see what I can do!

Looking forward to it!

[vaijns]

I've done some testing and it turns out, it actually does cause some problems. for Qwant the document.querySelector in the MutationObserver didn't work anymore and my current fix is pretty meh. And for brave the problem is the sidebar.checkVisibility() returning false at t he point where I'm doing it. I didn't get around to test the other two as I didn't want to sign up for Kagi and I didn't want to set up Searx right now. For the latter I might try it out in the future though.

So I'll definitely need to look into those issues and figure out how to fix them still.

[vaijns]

I've thought about (and started to try out) a possible refactor of the whole injection, which might make this a bit easier. It's based around the same model of each search engine having a list of possible queries for injection and also transformation functions based on the search engine and the injection location.

Some key differences are:

• The request is sent right away and also the whole script loads earlier as well (run_at in the manifest.json) which might decrease the delay (haven't measured it though)
  • I'll have to check if this works with Searx not using the query string but the actual input element
• The injection itself doesn't happen right after the response comes back but after the load event
• The query functions return Promises, so it's easier to deal with asynchronous things like using the MutationObserver

With those changes the flow might be a bit easier to understand because the MutationObserver and such things wouldn't be part of the main flow but specific to the search engine.
However it could also be more difficult to understand due to these same abstractions and the use of Promises.

I've pushed it to a separate branch if you want to take a look, though it's possible I'll make some bigger changes still. If that'd be too much change or for some other reason something you don't want, I'd totally understand of course.

[Fivefold]

I'm not opposed to this refactor, the injection started much smaller in scope for only two search engines originally and might not fit the current environment with ever more search engines anyway.

It will take me some time to work through it though, as I don't have as much free time as I've used to. Also, I understand there's still some TODOs left open when skimming over the code? Please notify me when it's tested with the different search engines and ready for review.

I would also appreciate some more in-code documentation about the structure and flow. You can look at the current searchInjection.js for the style I like, although you don't have to exactly copy it.

[vaijns]

I did address the TODOs (except a new one) and added some more documentation now and the general design is pretty much done. Though as there's still some problems I don't know how severe further changes will be, so probably not quite ready yet for a deeper review.

I've done some basic testing for Google, Duckduckgo, Brave and Qwant so far, though I definitely still need to do some more for those too. For Qwant it doesn't work fully yet when searching normally, it works only on full reloads of the page.
For Brave there's some odd behavior where I get an error saying injectionLocations and other constants would be redeclared, which is why I've wrapped the whole thing in an if for now. That's the remaining TODO I mentioned and I hope I'll find a proper fix for this.

Once I'm done I'll create a new PR for the other branch with screenshots for the full test matrix.