From 294facdb55d5d85351c5201cd617f76162ac3ba0 Mon Sep 17 00:00:00 2001 From: Mossroy Date: Wed, 30 May 2018 14:58:45 +0200 Subject: [PATCH 1/3] In ServiceWorker mode, set the iframe src to display articles. Instead of calling displayArticleInForm like in jQuery mode. I also removed some tests on jQuery/ServiceWorker modes in displayArticleInForm, as this function is now only used in jQuery mode. And I renamed it to displayArticleContentInIframe Fixes #382 --- www/js/app.js | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/www/js/app.js b/www/js/app.js index 70cb9a7da..96c4551c9 100644 --- a/www/js/app.js +++ b/www/js/app.js @@ -743,11 +743,26 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles * @param {DirEntry} dirEntry */ function readArticle(dirEntry) { - if (dirEntry.isRedirect()) { - selectedArchive.resolveRedirect(dirEntry, readArticle); + if (contentInjectionMode === 'serviceworker') { + // In ServiceWorker mode, we simply set the iframe src and show it when it's ready. + // (reading the backend is handled by the ServiceWorker itself) + var iframeArticleContent = document.getElementById('articleContent'); + iframeArticleContent.onload = function () { + iframeArticleContent.onload = function () {}; + // Actually display the iframe content + $("#readingArticle").hide(); + $("#articleContent").show(); + }; + iframeArticleContent.src = dirEntry.namespace + "/" + dirEntry.url; } else { - selectedArchive.readUtf8File(dirEntry, displayArticleInForm); + // In jQuery mode, we read the article content in the backend and manually insert it in the iframe + if (dirEntry.isRedirect()) { + selectedArchive.resolveRedirect(dirEntry, readArticle); + } + else { + selectedArchive.readUtf8File(dirEntry, displayArticleContentInIframe); + } } } @@ -825,15 +840,13 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles * @param {DirEntry} dirEntry * @param {String} htmlArticle */ - function displayArticleInForm(dirEntry, htmlArticle) { + function displayArticleContentInIframe(dirEntry, htmlArticle) { // Scroll the iframe to its top $("#articleContent").contents().scrollTop(0); - if (contentInjectionMode === 'jquery') { - // Replaces ZIM-style URLs of img, script and link tags with a data-url to prevent 404 errors [kiwix-js #272 #376] - // This replacement also processes the URL to remove the path so that the URL is ready for subsequent jQuery functions - htmlArticle = htmlArticle.replace(regexpTagsWithZimUrl, "$1data-kiwixurl$2$3"); - } + // Replaces ZIM-style URLs of img, script and link tags with a data-url to prevent 404 errors [kiwix-js #272 #376] + // This replacement also processes the URL to remove the path so that the URL is ready for subsequent jQuery functions + htmlArticle = htmlArticle.replace(regexpTagsWithZimUrl, "$1data-kiwixurl$2$3"); // Compute base URL var urlPath = regexpPath.test(dirEntry.url) ? urlPath = dirEntry.url.match(regexpPath)[1] : ''; @@ -862,15 +875,12 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles $("#articleContent").show(); // Allow back/forward in browser history pushBrowserHistoryState(dirEntry.namespace + "/" + dirEntry.url); - // If the ServiceWorker is not useable, we need to fallback to parse the DOM - // to inject images, CSS etc, and replace links with javascript calls - if (contentInjectionMode === 'jquery') { - parseAnchorsJQuery(); - loadImagesJQuery(); - loadCSSJQuery(); - //JavaScript loading currently disabled - //loadJavaScriptJQuery(); - } + + parseAnchorsJQuery(); + loadImagesJQuery(); + loadCSSJQuery(); + //JavaScript loading currently disabled + //loadJavaScriptJQuery(); }; // Load the blank article to clear the iframe (NB iframe onload event runs *after* this) From ce1e50de6672192eaa98d33be1c466dd264ae71b Mon Sep 17 00:00:00 2001 From: Jaifroid Date: Wed, 30 May 2018 17:02:44 +0100 Subject: [PATCH 2/3] Load cached assets into html string and prevent render till CSS fulfilled --- www/js/app.js | 49 ++++++++++++++++++++++++++------------------ www/js/init.js | 4 ++++ www/js/lib/uiUtil.js | 38 ++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/www/js/app.js b/www/js/app.js index 96c4551c9..8ea1a4984 100644 --- a/www/js/app.js +++ b/www/js/app.js @@ -544,8 +544,8 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles */ function resetCssCache() { // Reset the cssCache. Must be done when archive changes. - if (cssCache) { - cssCache = new Map(); + if (assetsCache) { + assetsCache = new Map(); } } @@ -829,10 +829,6 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles // DEV: If you want to support more namespaces, add them to the END of the character set [-I] (not to the beginning) var regexpTagsWithZimUrl = /(<(?:img|script|link)\s+[^>]*?\b)(?:src|href)(\s*=\s*["']\s*)(?:\.\.\/|\/)+([-I]\/[^"']*)/ig; - // Cache for CSS styles contained in ZIM. - // It significantly speeds up subsequent page display. See kiwix-js issue #335 - var cssCache = new Map(); - /** * Display the the given HTML article in the web page, * and convert links to javascript calls @@ -857,6 +853,13 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles // Extract any css classes from the html tag (they will be stripped when injected in iframe with .innerHTML) var htmlCSS = htmlArticle.match(/]*class\s*=\s*["']\s*([^"']+)/i); htmlCSS = htmlCSS ? htmlCSS[1] : ''; + + // Check if any of the CSS is already in the cache, and if so, insert it in the html string + uiUtil.replaceCSSLinksInHtml(htmlArticle, 'data-kiwixurl', + function(result) { + htmlArticle = result; + } + ); // Tell jQuery we're removing the iframe document: clears jQuery cache and prevents memory leaks [kiwix-js #361] $('#articleContent').contents().remove(); @@ -870,9 +873,8 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles articleContent.innerHTML = htmlArticle; // Add any missing classes stripped from the tag if (htmlCSS) articleContent.getElementsByTagName('body')[0].classList.add(htmlCSS); - // Actually display the iframe content - $("#readingArticle").hide(); - $("#articleContent").show(); + // Display the article if CSS is fulfilled + isCSSFulfilled(); // Allow back/forward in browser history pushBrowserHistoryState(dirEntry.namespace + "/" + dirEntry.url); @@ -956,31 +958,38 @@ define(['jquery', 'zimArchiveLoader', 'util', 'uiUtil', 'cookies','abstractFiles for (var i = collapsedBlocks.length; i--;) { collapsedBlocks[i].classList.add('open-block'); } - - $('#articleContent').contents().find('link[data-kiwixurl]').each(function() { + $('#articleContent').contents().find('link[data-kiwixurl]').each(function () { var link = $(this); var linkUrl = link.attr("data-kiwixurl"); var title = uiUtil.removeUrlParameters(decodeURIComponent(linkUrl)); - if (cssCache && cssCache.has(title)) { - var cssContent = cssCache.get(title); - uiUtil.replaceCSSLinkWithInlineCSS(link, cssContent); - } else { - selectedArchive.getDirEntryByTitle(title) + selectedArchive.getDirEntryByTitle(title) .then(function (dirEntry) { return selectedArchive.readUtf8File(dirEntry, function (fileDirEntry, content) { - var fullUrl = fileDirEntry.namespace + "/" + fileDirEntry.url; - if (cssCache) cssCache.set(fullUrl, content); - uiUtil.replaceCSSLinkWithInlineCSS(link, content); + var fullUrl = fileDirEntry.namespace + "/" + fileDirEntry.url; + if (assetsCache) assetsCache.set(fullUrl, content); + uiUtil.replaceCSSLinkWithInlineCSS(link, content); + assetsCache.cssFulfilled++; + isCSSFulfilled(); } ); }).fail(function (e) { console.error("could not find DirEntry for CSS : " + title, e); + assetsCache.cssCount--; + isCSSFulfilled(); }); - } }); } + // Some pages are extremely heavy to render, so we prevent rendering by keeping the iframe hidden + // until all CSS content is available [kiwix-js #381] + function isCSSFulfilled() { + if (assetsCache.cssFulfilled >= assetsCache.cssCount) { + document.getElementById('readingArticle').style.display = 'none'; + document.getElementById('articleContent').style.display = 'block'; + } + } + function loadJavaScriptJQuery() { $('#articleContent').contents().find('script[data-kiwixurl]').each(function() { var script = $(this); diff --git a/www/js/init.js b/www/js/init.js index 1a76ec4c9..8fb174431 100644 --- a/www/js/init.js +++ b/www/js/init.js @@ -22,6 +22,10 @@ */ 'use strict'; +// Provides caching for CSS styles contained in ZIM (variable needs to be available app-wide) +// It significantly speeds up subsequent page display. See kiwix-js issue #335 +var assetsCache = new Map(); + require.config({ baseUrl: 'js/lib', paths: { diff --git a/www/js/lib/uiUtil.js b/www/js/lib/uiUtil.js index 6c261c6fa..18eccb988 100644 --- a/www/js/lib/uiUtil.js +++ b/www/js/lib/uiUtil.js @@ -72,6 +72,43 @@ define([], function() { } link.replaceWith(cssElement); } + + /** + * Replaces all CSS links that have the given attribute in the html string with inline script tags containing content + * from the cache entries. Returns the substituted html in the callback function (even if no substitutions were made). + * + * @param {String} html The html string to process + * @param {String} attribute The attribute that stores the URL to be substituted + * @param {Function} callback The function to call with the substituted html + + }} + */ + function replaceCSSLinksInHtml(html, attribute, callback) { + // This regex creates an array of all link tags that have the given attribute + var regexpLinksWithAttribute = new RegExp(']+?' + attribute + '=["\']([^"\']+)[^>]*>', 'ig'); + var titles = []; + var linkArray = regexpLinksWithAttribute.exec(html); + while (linkArray !== null) { + // Store both the link to be replaced and the decoded URL + titles.push([linkArray[0], + decodeURIComponent(linkArray[1])]); + linkArray = regexpLinksWithAttribute.exec(html); + } + assetsCache.cssCount = 0; + assetsCache.cssFulfilled = 0; + titles.forEach(function(title) { + assetsCache.cssCount++; + var cssContent = assetsCache.get(title[1]); + if (cssContent) { + assetsCache.cssFulfilled++; + html = html.replace(title[0], + ''); + } + if (assetsCache.cssCount >= titles.length) { + callback(html); + } + }); + } var regexpRemoveUrlParameters = new RegExp(/([^?#]+)[?#].*$/); @@ -90,6 +127,7 @@ define([], function() { return { feedNodeWithBlob: feedNodeWithBlob, replaceCSSLinkWithInlineCSS: replaceCSSLinkWithInlineCSS, + replaceCSSLinksInHtml: replaceCSSLinksInHtml, removeUrlParameters: removeUrlParameters }; }); From a3d51c4c6b2dd99bb6e4b2426749dedf2efa3184 Mon Sep 17 00:00:00 2001 From: Jaifroid Date: Wed, 30 May 2018 17:59:22 +0100 Subject: [PATCH 3/3] Minor changes in comments --- www/js/init.js | 2 +- www/js/lib/uiUtil.js | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/www/js/init.js b/www/js/init.js index 8fb174431..4b447886b 100644 --- a/www/js/init.js +++ b/www/js/init.js @@ -22,7 +22,7 @@ */ 'use strict'; -// Provides caching for CSS styles contained in ZIM (variable needs to be available app-wide) +// Provides caching for assets contained in ZIM (variable needs to be available app-wide) // It significantly speeds up subsequent page display. See kiwix-js issue #335 var assetsCache = new Map(); diff --git a/www/js/lib/uiUtil.js b/www/js/lib/uiUtil.js index 18eccb988..dd0e4589d 100644 --- a/www/js/lib/uiUtil.js +++ b/www/js/lib/uiUtil.js @@ -80,8 +80,6 @@ define([], function() { * @param {String} html The html string to process * @param {String} attribute The attribute that stores the URL to be substituted * @param {Function} callback The function to call with the substituted html - - }} */ function replaceCSSLinksInHtml(html, attribute, callback) { // This regex creates an array of all link tags that have the given attribute