kiwix-js icon indicating copy to clipboard operation
kiwix-js copied to clipboard

Add downloadable fonts (from ZIM) to files processed in jQuery mode

Open Jaifroid opened this issue 2 years ago • 26 comments

While we said we would not optimize jQuery mode any further, this looks like a bugfix. Recent Gutenberg ZIMs seem to rely on font files donwloaded form the ZIM to produce the PDF / EPUB / HTML icons. Since we don't process these as we do images, there are now empty placeholders where the icons should be. See screenshot and note the corresponding "donwloadable font" errors in the console. This shouldn't be too hard to fix within our existing processing of images or possibly media. It would be a case of converting these URLs to blob URLs. This isn't technically a regression in our code, but it is a regression in usability of Gutenberg archives in jQuery mode.

image

Jaifroid avatar Jan 04 '22 21:01 Jaifroid

@Jaifroid I would like to work on this issue, but I'm not sure how to proceed :sweat_smile:

ankur12-1610 avatar Jan 10 '22 05:01 ankur12-1610

I don't think anything has changed recently on this. We always have used font-awesome for these icons.

kelson42 avatar Jan 10 '22 05:01 kelson42

Well it's possible we've only just noticed an issue that has existed for a while, but I'm 99% sure that old Gutenberg ZIMs were able to show icons (they are quite prominent, and I have worked a lot on Gutenberg ZIMs supporting downloadable books before, so I would have noticed such a glaring issue). Now they can't display the icons (in Kiwix JS), And in any case, we don't have any code to extract these fonts from the ZIM in jQuery mode, which is what is needed here. So it's worth working on this despite the fact that it works natively in SW mode, as it is really just a small addition to the image download code, I think.

Jaifroid avatar Jan 10 '22 06:01 Jaifroid

@ankur12-1610 I've assigned you -- thanks.

Jaifroid avatar Jan 10 '22 06:01 Jaifroid

While we said we would not optimize jQuery mode any further, this looks like a bugfix. Recent Gutenberg ZIMs seem to rely on font files donwloaded form the ZIM to produce the PDF / EPUB / HTML icons. Since we don't process these as we do images, there are now empty placeholders where the icons should be. See screenshot and note the corresponding "donwloadable font" errors in the console. This shouldn't be too hard to fix within our existing processing of images or possibly media. It would be a case of converting these URLs to blob URLs. This isn't technically a regression in our code, but it is a regression in usability of Gutenberg archives in jQuery mode.

Forgot to approve to work on this if you wish

mossroy avatar Jan 10 '22 06:01 mossroy

@ankur12-1610 I've assigned you -- thanks.

Thanks, I'll start working on it :smile:

ankur12-1610 avatar Jan 10 '22 08:01 ankur12-1610

While we said we would not optimize jQuery mode any further, this looks like a bugfix. Recent Gutenberg ZIMs seem to rely on font files donwloaded form the ZIM to produce the PDF / EPUB / HTML icons. Since we don't process these as we do images, there are now empty placeholders where the icons should be. See screenshot and note the corresponding "donwloadable font" errors in the console. This shouldn't be too hard to fix within our existing processing of images or possibly media. It would be a case of converting these URLs to blob URLs. This isn't technically a regression in our code, but it is a regression in usability of Gutenberg archives in jQuery mode.

Forgot to approve to work on this if you wish

Yeah sure :+1:

ankur12-1610 avatar Jan 10 '22 08:01 ankur12-1610

@ankur12-1610 For information, since #771 was merged, the app caches its own code in appCache. This means that if you test code using an http-server, you will need to empty the cache after you've changed some code and before you reload the app, in order to see your new code. In Dev Tools, you can do this from the Application tab in Chromium, or from the Storage tab in Firefox. Under "Cache Storage" you should see two caches. One of these is kiwix-js-appCache-3.x.x... . Right click this and choose "delete", then reload the app. You may be able to reload caches automatically by checking, under Service Workers, the option "Update on reload" (in Chromium), but don't forget to turn this option off to test the code under normal conditions. I'm not sure it always works either (it may not empty the cache, only update the SW).

Jaifroid avatar Jan 10 '22 09:01 Jaifroid

@ankur12-1610 For information, since #771 was merged, the app caches its own code in appCache. This means that if you test code using an http-server, you will need to empty the cache after you've changed some code and before you reload the app, in order to see your new code. In Dev Tools, you can do this from the Application tab in Chromium, or from the Storage tab in Firefox. Under "Cache Storage" you should see two caches. One of these is kiwix-js-appCache-3.x.x... . Right click this and choose "delete", then reload the app. You may be able to reload caches automatically by checking, under Service Workers, the option "Update on reload" (in Chromium), but don't forget to turn this option off to test the code under normal conditions. I'm not sure it always works either (it may not empty the cache, only update the SW).

Got it! will keep it in mind whenever I'll test the code.

ankur12-1610 avatar Jan 10 '22 10:01 ankur12-1610

@Jaifroid is there a source where I can get the zim file of the book present in the screenshot, I did visit all the websites and used also used openzim/gutenberg which isn't working properly in my case. Could you tell me the source from which you downloaded it?:sweat_smile:

ankur12-1610 avatar Jan 17 '22 14:01 ankur12-1610

@ankur12-1610 Any of the recent Gutenberg ZIMs seem to have this issue: see http://download.kiwix.org/zim/gutenberg/ . I've just downloaded and tested http://download.kiwix.org/zim/gutenberg/gutenberg_it_all_2022-01.zim. You'll only see the issue in JQuery mode. The font-based icons work in Service Worker mode. If these are in fact the same font files and scripts that we use for our Bootstrap installation, then maybe we don't need to extract them from the ZIM and attach them (this is a bit like what we did with the WebHero WebP polyfill, we just use our own version rather than the one provided in the ZIM).

Jaifroid avatar Jan 17 '22 16:01 Jaifroid

To me, it's not the same situation as with the WebP polyfill. For WebP, the polyfill might not be present inside the ZIM file. Providing ours was a good way to make sure it would work in all cases. For the fonts, they are in the ZIM, and might be different in each ZIM file : I think we should always read them in the ZIM, just like we do for images and CSS

mossroy avatar Jan 17 '22 17:01 mossroy

@mossroy OK! I was thinking it might involve extracting a whole Bootstrap version from the ZIM and running its JavaScript, which might be tricky, but to be honest I haven't actually investigated how the fonts work in the newer Gutenberg archives. If it's pure CSS, it would be doable.

Jaifroid avatar Jan 17 '22 17:01 Jaifroid

I did not check for Gutenberg but, usually, fonts are another type of file (not CSS) : .ttf, .woff and maybe other ones.

In this case, what would be needed is to do roughly the same as what we do for images and CSS : parse the DOM for references of font files, read them in the ZIM file, and inject the blobs in the DOM

Hopefully this change would be accepted by the browsers after the page is loaded, so that they replace the strange signs by the good ones on-the-fly. It has to be tested to check it works

mossroy avatar Jan 17 '22 17:01 mossroy

It's definitely font-awesome -- see screenshot below. I only mentioned Bootstrap, because font-awesome is included in Bootstrap, but I've just checked and the Gutenberg pages don't seem to be using Bootstrap. It's done in CSS. The CSS has embedded font data, but they don't seem to be rendering. The CSS looks like:

@font-face {
  font-family: 'FontAwesome';
  src: url('data:application/vnd.ms-fontobject;base64,ARsBABsaAQACAA..................
  font-weight: normal;
  font-style: normal;
}

image

Jaifroid avatar Jan 17 '22 17:01 Jaifroid

OK I now understand why you were mentioning CSS. You're right. But the first screenshot of the issue also shows missing .ttf and .woff files (maybe it's a different issue)

mossroy avatar Jan 17 '22 17:01 mossroy

Looking more closely at it, it's not a separate issue. I'm definitely getting missing downloadable fonts relating to font-awesome appearing when I run it in the Chromium extension, in the same ZIM as above, in JQuery mode (see screenshot below) so clearly it isn't only pure CSS, or it isn't always pure CSS embedded. I wonder if downloading them is a fallback? Needs investigating...

image

Jaifroid avatar Jan 17 '22 18:01 Jaifroid

I'm a bit stunned to see this when testing #807 in Internet Explorer mode:

image

How come the icons are displaying correctly in IE, but not in other browsers? 😕

Jaifroid avatar Jan 29 '22 11:01 Jaifroid

Something very wrong with the CSS extracted here (this is in Chromium-Edge):

image

Jaifroid avatar Jan 29 '22 12:01 Jaifroid

I'm a bit stunned to see this when testing #807 in Internet Explorer mode:

image

How come the icons are displaying correctly in IE, but not in other browsers? 😕

Wow😲

ankur12-1610 avatar Jan 29 '22 12:01 ankur12-1610

Something very wrong with the CSS extracted here (this is in Chromium-Edge):

image

@Jaifroid I'm stuck at a certain point ig, what changes should I do in the code 🤔?

ankur12-1610 avatar Jan 29 '22 12:01 ankur12-1610

@ankur12-1610 Without knowing what you've tried, it's hard to know how to help!

Jaifroid avatar Jan 29 '22 16:01 Jaifroid

Something very wrong with the CSS extracted here (this is in Chromium-Edge):

image

This is a completely different issue. We accidentally extract <link rel="shortcut icon" href=".../favicon.ico"> as if it were a stylesheet. I'll create separate issue.

Jaifroid avatar Jan 29 '22 18:01 Jaifroid

This is related to #338 and #747.

Jaifroid avatar Jan 31 '22 22:01 Jaifroid

This is related to #338 and #747.

@Jaifroid really sorry for the late reply was off for 2 days due to illness 😢

ankur12-1610 avatar Feb 01 '22 03:02 ankur12-1610

This is related to #338 and #747.

Got it :+1:

ankur12-1610 avatar Feb 01 '22 03:02 ankur12-1610