fava icon indicating copy to clipboard operation
fava copied to clipboard

Fava extensions with <script> tags

Open hardikar opened this issue 4 years ago • 10 comments

I wanted to write a fava extension with some dynamic logic which I put inside a <script> tag in my template .html file.

It works well if I load the the extension directly using the url. However, it I click on the extension link in the sidebar (while somewhere else in Fava), it seems to ignores the <script> tag completely.

After some digging around, I think it's because of the way Fava intercepts all clicks on links and will in most cases asynchronously load the content of the page and replace the <article> contents with them.

More specifically, I think, it is because of this code in loadURL:

      const article = document.querySelector("article");
      if (article) {
        article.innerHTML = content;
      }

Googling around a bit, it turns out that script elements inserted using innerHTML do not execute when they are inserted. (see https://www.w3.org/TR/2008/WD-html5-20080610/dom.html#innerhtml0).

Although I'm not very familiar with HTML/JS, but I think one solution to this is to use document.write() instead of setting innerHTML. (See https://developer.mozilla.org/en-US/docs/Web/API/Document/write)

Currently, I'm using a workaround (that I'm not a super fan of):

<img src="1" style="display:none" onerror='eval(document.getElementById("myscript").innerHTML);'>

which works, by forcing a an eval, which may be even worse than using innerHTML.

I hope this is something that can be fixed fairly easily, because it'll really increase the functionality of Fava's extension framework.

hardikar avatar Nov 01 '20 17:11 hardikar

I wanted to write a fava extension with some dynamic logic

This is definitely something that Fava should support :+1:

If you look at all the notes on the MDN page for document.write(), there's some that mean that this also won't work for us just like setting innerHTML (the Javascript is loaded as a deferred script). It would also seem like a bit of a hack to me.

I think the more correct way to do this would be for the extension to include a .js file (which should be a Javascript module) This JS module should have a named export called, e.g., initExtension (there's probably a better name for it) which should be a function to run on page load. Using an async import() we could then import this on page load of the extension.

yagebu avatar Nov 14 '20 14:11 yagebu

What type of <script> are you using? I've got an extension which uses a few javascript functions and I haven't had any issues with those working.

floatingheads avatar Jan 25 '21 16:01 floatingheads

<script id="thing" type="text/javascript">
  <!-- code defining some JS functions -->
</script>
<!-- Hack to load script tag, when FAVA uses article.innerHTML for async loading-->
<img src="1" style="display:none" onerror='eval(document.getElementById("thing").innerHTML);'>

It works great when I refresh the page (i.e connect directly to the page), but not when I click on the extension on the left bar, and the HTML is loaded dynamically (using innerHTML as described in the issue).

@floatingheads Would you mind sharing how you use it then?

hardikar avatar Feb 14 '21 18:02 hardikar

@yagebu

I think the more correct way to do this would be for the extension to include a .js file

That sounds great! I would be happy to help implement this, if you like. Unfortunately, I don't have a lot of JS experience, mostly C/C++/Python. So if you can point me to the right documentation and if possible, an example implementation of what you're thinking, I'll be happy to do some research and can probably work on it to open a PR.

hardikar avatar Feb 14 '21 18:02 hardikar

@hardikar: Thank you for your interest in helping with this! MDN is probably a good starting point for a better understanding of what Javascript modules mean. I'd imagine we'd want the plugin to provide a JS file that roughly looks like the following:

export default {
    onExtensionPageLoad() {  # the exact name for these "hooks" is tbd.
         // Do stuff that you want to do when the extension page load.s
    },
    otherExtensionHook() {
        // ... do other stuff - maybe we'd have a hook for every page load, not just on the extension.
    }
}

As you don't have a lot of JS experience, I can handle the integration of this sort of extension into the frontend (importing it and ensuring the supplied functions are called at the right times). But there's a couple of other things we need: The (Python) extensions somehow need to specify that they include Javascript (like the template this could be at some canonical path relative to the extension) and then we need to expose those modules at some URL and provide a list of extensions to the frontend so that it can know which extensions it needs to import.

yagebu avatar Feb 20 '21 17:02 yagebu

@yagebu That sounds good and thanks for outlining what needs to be done. I'll take a look at this soon.

hardikar avatar Mar 16 '21 00:03 hardikar

@hardikar thank you for the hacky workaround with an <img> onerror attribute, I have resorted to using the same until this issue can be fixed.

vurguuz avatar Apr 28 '21 11:04 vurguuz

Here's another workaround that supports multiple <script> tags on the page.

<img src="1" style="display:none"
    onerror='if(!window.location.hash){window.location=window.location + "#loaded"; window.location.reload()}'>

gerdemb avatar Oct 23 '21 15:10 gerdemb

I think an easy solution for this problem would be if all links to fava extension pages have the data-remote attribute, which prevents fava from intercepting the link: https://github.com/beancount/fava/blob/212c86adb2c6f268845384737efd446b743e5342/frontend/src/router.ts#L187-L207

This has the advantage that also

In general, what's the original purpose of the interception of all links, loading it with ajax and replacing the element in the DOM? To reduce network traffic by not transferring the entire HTML to the browser? To remove some (possible?) flickering of the navigation bar, when navigating to new sites?

andreasgerstmayr avatar Oct 23 '21 15:10 andreasgerstmayr

I see that when clicking on the extension links in the navigation sidebar after adding the data-remote attribute to them the filters from the top of the page (Time, Account, Filter by tag, payee, ...) are not applied on the extension pages. So this approach would not properly solve the issue, I believe.

vurguuz avatar Nov 18 '21 08:11 vurguuz