Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make zim's articles insert itself client specific js. #392

Open
mgautierfr opened this issue Jul 27, 2020 · 18 comments
Open

Make zim's articles insert itself client specific js. #392

mgautierfr opened this issue Jul 27, 2020 · 18 comments

Comments

@mgautierfr
Copy link
Member

(I'm not sure where to create this issue, I keep it in the same repository of the original comment)

We could use a small js file in every article to modify the dom. This small (or not) script would always be the same name, a wellknown name (ie, /clientScript.js or something like that). Even if all articles in the zim file would try to include it, it would be not in the zim file, but provided by the client (kiwix-tools, android, macos, desktop client, ...). It would be to the js to modify the dom to add the taskbar, do ask for suggestions and so.

I see several pros for this:

  • If we add the script tag in the html directly, we don't have to modify the html on the server to include the taskbar.
  • The js/css would be cached by the browser, instead of being download several times (embedded in the html)
  • The js could do more than add the taskbar, on native client, it could be use to parse the html, generate a table of content and send it to the native code using the appropriate way.
  • Vendors, who already proxy kiwix-serve, could intercept the request to the js and provide their own. This way, they could use custom css, button, ... to integrate correctly with their system.

The main cons I see is that we need js compatible browser. This is the case for most of the browser in the market but some vendors may use
pretty old browser or specific device that doesn't support js (I'm thinking about reader/tablet). We should see with them if this is a blocker or not (@tim-moody, @letompouce, other...)

Originally posted by @mgautierfr in #322 (comment)

@kelson42
Copy link
Collaborator

I believe in using and external JS to introduce the taskbar (so most of the code is in a an external JS resource/URI, but the triggering of the event is still hardcoded in the DOM via C++ in kiwix-serve) but I don't believe that creating a reverse-JS-API is a good idea. This is elegant but put a constraint on the the ZIM creators which is not worth it. ZIM readers anyway read/write the DOM, this will be in particular true when the ZIM JS API will be there, so this does not really matter. In term of use case, we have only one case AFAIK, which is kiwix-serve (where we don't have really a control of the DOM outside the C++).

@mgautierfr
Copy link
Member Author

Comment from @tim-moody on previous issue :

If I understand correctly you are proposing a js wrapper, external to the zim content, which sounds like a good idea. I would call it kiwixClientScript.js or something less generic. We have struggled with browser compatibility and have gradually moved towards ECMA2015 or some such. The main thing we allow is flexbox.

@mgautierfr
Copy link
Member Author

Comment from @rgaudin on previous issue :

I also think @mgautierfr's suggestion is good. It would add a lot of flexibility and could foster contributions to this part which has received many requests over the years but is currently not that easy to hack on.

I'm not in favor of dropping no-js support though ; so that would mean injecting content to the page anyway…

  • display the HTML-only part (what's currently in body .kiwix) of the taskbar (or even a simplified version) in a tag.
  • display a link plus css code hiding main content inside a tag. the link would lead to that same page but letting libkiwix know it should include an oldstyle taskbark.

Not in favor of the latter as it would mean two different URLs for the same content.

What I don't like about any version is that it means two different ways to render the taskbar with one that we'll always forget to test and probably break.

@mgautierfr
Copy link
Member Author

This is elegant but put a constraint on the the ZIM creators which is not worth it

The only constraints would be to insert <script type="text/javascript" src="kiwixClientScript.js"></script> in the head section.

ZIM readers anyway read/write the DOM

I don't know about android and ios/macos, but kiwix-desktop don't. From what I know, only kiwix-serve does it (to introduce the taskbar and the external links blocker).
And it is a performance issue (modification of strings/reallocation and sending several time the same content).

In term of use case, we have only one case AFAIK, which is kiwix-serve (where we don't have really a control of the DOM outside the C++).

That's true.
Other tools could use it but this is not a real use case as most of them control the browser and can insert js directly. But it could simplify the implementation of such feature.
I don't know if it could be useful for other projects (https://github.com/kiwix/kiwix-js)

@kelson42
Copy link
Collaborator

kelson42 commented Jul 27, 2020

This is elegant but put a constraint on the the ZIM creators which is not worth it

The only constraints would be to insert <script type="text/javascript" src="kiwixClientScript.js"></script> in the head section.

Yes. But this is more complex than this, because depending where you put that <script>, behaviours will be different, in fact readers devs probably need more control than just a <script> in the DOM. But without concrete tickets with problems/bugs, this dicussion is really difficult.

ZIM readers anyway read/write the DOM

I don't know about android and ios/macos, but kiwix-desktop don't. From what I know, only kiwix-serve does it (to introduce the taskbar and the external links blocker).
And it is a performance issue (modification of strings/reallocation and sending several time the same content).

They all do it except Kiwix-Desktop indeed, and this is missing, see for example kiwix/kiwix-desktop#42 or
kiwix/kiwix-desktop#320

In term of use case, we have only one case AFAIK, which is kiwix-serve (where we don't have really a control of the DOM outside the C++).

That's true.
Other tools could use it but this is not a real use case as most of them control the browser and can insert js directly. But it could simplify the implementation of such feature.
I don't know if it could be useful for other projects (https://github.com/kiwix/kiwix-js)

AFAIK, I'm not aware about any ticket which would be solved by this, therefore I still believe we can agree that this is still better to have one <script...> insert in kiwix-serve code than requiring all ZIM in all pages to made one.

@rgaudin
Copy link
Member

rgaudin commented Jul 27, 2020

We could also introduce a metadata at ZIM level indicating to not touch the DOM and in this case it's the content that's responsible to inject that commonly-named JS file or not.
That would provide the benefit of this solution without the requirement of including it for all ZIM creators. But those who know what they're doing can save this parsing step and the cons that comes with it.

@mgautierfr
Copy link
Member Author

We could also introduce a metadata at ZIM level indicating to not touch the DOM and in this case it's the content that's responsible to inject that commonly-named JS file or not.

It would be necessary anyway to allow the client to know if it must add the taskbar (old zim) or not.

@mgautierfr
Copy link
Member Author

Yes. But this is more complex than this, because depending where you put that <script>, behaviours will be different, in fact readers devs probably need more control than just a <script> in the DOM.

Not really, it would be js injected, it would be to the js to adapt the DOM. There is no real impact on where the the script is inserted (and if it is important, I prefer to have the wrapper inserting it where it make sense instead of the reading try to insert it at the right place).

The real point is about the relative url of the script. We could enforce that the url is relative (a must have) but point to the root of the zim file (and so wrapper has to be a bit smart).
Or just put the script always at the same level than the article (and so the reader has to catch the url and provide the content, whatever is the path of the file).

But without concrete tickets with problems/bugs, this dicussion is really difficult.

This is the ticket.

They all do it except Kiwix-Desktop indeed, and this is missing, see for example kiwix/kiwix-desktop#42 or kiwix/kiwix-desktop#320

There is no need to modify the content. For TOC we could simply read the content, not modify it (without speaking of the js api discussed in openzim/mwoffliner#1237). For darkmode, we need a custom CSS, and the qtwebengine exeample does it without modify the content, it inject a js to add the css : https://doc.qt.io/qt-5/qtwebengine-webenginewidgets-stylesheetbrowser-example.html

AFAIK, I'm not aware about any ticket which would be solved by this

#391 ?
Performance ? Maintainability of the code (no need to parse html with regex) ?

@tim-moody
Copy link

The main cons I see is that we need js compatible browser.

We are pretty far down the road of assuming js in the browser. Our whole menuing system is done in js on the client. Even on OLPC XOs there is js, though not the most recent. Maybe @holta can add more.

@kelson42
Copy link
Collaborator

The main cons I see is that we need js compatible browser.

We are pretty far down the road of assuming js in the browser. Our whole menuing system is done in js on the client. Even on OLPC XOs there is js, though not the most recent. Maybe @holta can add more.

I believe I share this opinion.

@tim-moody
Copy link

@kelson42 not sure why you closed this. Is it rejected as an enhancement?

@mgautierfr mgautierfr reopened this Jul 27, 2020
@mgautierfr
Copy link
Member Author

mgautierfr commented Jul 27, 2020

I think that @tim-moody said we can almost assume we have js in the browser. Their whole menuing system is done in js on the client side.

@rgaudin
Copy link
Member

rgaudin commented Jul 27, 2020

#322 as well but I'm not looking much at bugs it could fix rather than what it enables:

  • Easy custom branding
  • Easy internationalization of the taskbar
  • Easy custom link blocking

@tim-moody
Copy link

at what point would the added js script gain control?

@tim-moody

This comment has been minimized.

@mgautierfr
Copy link
Member Author

at what point would the added js script gain control?

As any js script : Gain full control on the DOM and can change whatever it want. (Limited by crossdomain access authorization)

@mgautierfr

This comment has been minimized.

@stale
Copy link

stale bot commented Sep 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants