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

Font size option (user preference) does not work when defined explicitly in author-provided CSS #34

Closed
danielweck opened this issue Jul 8, 2014 · 10 comments

Comments

@danielweck
Copy link
Member

danielweck commented Jul 8, 2014

Originally posted by
https://github.com/vincent-gros
at
readium/readium-js-viewer#47

When the CSS file including in the EPUB file has explicit font size values like:
.medium { font-size: medium; }
The font size change option doesn't work.
This issue seems to be related to some functions in lib/ReaderSetingsDialog.js.
To see the problem, I made a sample, avaiable here.

@edas
Copy link

edas commented Jul 9, 2014

One possible fix is : each time the user change the font size and each time we load a chapter, we iterate through all CSS rules to find all font-size [xx-small,x-small,small,medium,large,x-large,xx-large] and replace them by the corresponding rem value :

absolute value rem value
xx-small 0.5625rem
x-small 0.6250rem
small 0.8125rem
medium 1rem
large 1.125rem
x-large 1.5rem
xx-large 2rem

However, this won't be perfect. If there is a dynamically loaded CSS or a script changing a style attribute after we parse the CSS rules, these sizes will stay as-is.

@mickael-menu-mantano
Copy link

Another solution would be to use – when available – the text-size-adjust
CSS property, instead of changing directly the font size.

https://developer.mozilla.org/en-US/docs/Web/CSS/text-size-adjust

An example working on iOS and probably Android:
epubDocument.body.style.webkitTextSizeAdjust = "200%";

However it seems to be only available on mobile devices, but maybe there is
an equivalent for desktops.

Mickaël Menu
R&D Engineer

2 rue du Helder - 75009 Paris, France
Tel.: +33 1 42 47 05 61

mmenu@mantano.com
www.mantano.com
store.mantano.com
cloud.mantano.com

On Wed, Jul 9, 2014 at 3:51 PM, Éric D. notifications@github.com wrote:

One possible fix is : each time the user change the font size and each
time we load a chapter, we iterate through all CSS rules to find all
font-size [xx-small,x-small,small,medium,large,x-large,xx-large] and
replace them by the corresponding rem value :
absolute value rem value xx-small 0.5625rem x-small 0.6250rem small
0.8125rem medium 1rem large 1.125rem x-large 1.5rem xx-large 2rem

However, this won't be perfect. If there is a dynamically loaded CSS or a
script changing a style attribute after we parse the CSS rules, these
sizes will stay as-is.


Reply to this email directly or view it on GitHub
#34 (comment)
.

@danielweck
Copy link
Member Author

@edas
Copy link

edas commented Jul 14, 2014

Hi. I am afraid not to have an account on the bugzilla, and I don't see how to create one. Could you help me on that ?

@danielweck
Copy link
Member Author

Could you please explain why you would need a Bugzilla account?
(development issues are tracked on GitHub) Regards, Daniel

On Monday, July 14, 2014, Éric D. notifications@github.com wrote:

Hi. I am afraid not to have an account on the bugzilla, and I don't see
how to create one. Could you help me on that ?


Reply to this email directly or view it on GitHub
#34 (comment)
.

@edas
Copy link

edas commented Jul 14, 2014

I don't know, as I don't know what is on the bugzilla.

You posted a reference link that I couldn't follow and that seemed to be related to the issue or its resolution. I don't have any other information : I had the assumption that the work continued there, but I may be wrong (as I don't see anything on the bugzilla).

@danielweck
Copy link
Member Author

Ah, apologies :)
(my email thread doesn't show this part, I had to go on GitHub to be
reminded of the link you are referring to)

Please do not worry about the Bugzilla link, this is for internal tracking
purposes only. The issues are recorded and processed in GitHub.

Sorry for the confusion.
Regards, Daniel

On Monday, July 14, 2014, Éric D. notifications@github.com wrote:

I don't know, as I don't know what is on the bugzilla.

You posted a reference link that I couldn't follow and that seemed to be
related to the issue or its resolution. I don't have any other information
: I had the assumption that the work continued there, but I may be wrong
(as I don't see anything on the bugzilla).


Reply to this email directly or view it on GitHub
#34 (comment)
.

@danielweck
Copy link
Member Author

I added a comment in the Git diff, as unfortunately some text elements are not resizing. See:
259126f

@danielweck
Copy link
Member Author

Follow-up of the above comment (so we can track commits for this issue):
afabcf1

@rkwright rkwright modified the milestones: m1.2, m1.1 Oct 24, 2014
@danielweck
Copy link
Member Author

danielweck commented Oct 28, 2014

Unless I am mistaken, this issue was fixed by @ryanackley 's code:
https://github.com/readium/readium-shared-js/blob/develop/js/helpers.js#L242

Helpers.UpdateHtmlFontSize = function ($epubHtml, fontSize) {

    var factor = fontSize / 100;
    var win = $epubHtml[0].ownerDocument.defaultView;
    var $textblocks = $('p, div, span, h1, h2, h3, h4, h5, h6, li, blockquote, td, pre', $epubHtml);
    var originalLineHeight;


    // need to do two passes because it is possible to have nested text blocks.
    // If you change the font size of the parent this will then create an inaccurate
    // font size for any children.
    for (var i = 0; i < $textblocks.length; i++) {
        var ele = $textblocks[i],
            fontSizeAttr = ele.getAttribute('data-original-font-size');

        if (!fontSizeAttr) {
            var style = win.getComputedStyle(ele);
            var originalFontSize = parseInt(style.fontSize);
            originalLineHeight = parseInt(style.lineHeight);

            ele.setAttribute('data-original-font-size', originalFontSize);
            // getComputedStyle will not calculate the line-height if the value is 'normal'. In this case parseInt will return NaN
            if (originalLineHeight) {
                ele.setAttribute('data-original-line-height', originalLineHeight);
            }
        }
    }

    // reset variable so the below logic works. All variables in JS are function scoped.
    originalLineHeight = 0;
    for (var i = 0; i < $textblocks.length; i++) {
        var ele = $textblocks[i],
            fontSizeAttr = ele.getAttribute('data-original-font-size'),
            lineHeightAttr = ele.getAttribute('data-original-line-height'),
            originalFontSize = Number(fontSizeAttr);

        if (lineHeightAttr) {
            originalLineHeight = Number(lineHeightAttr);
        }
        else {
            originalLineHeight = 0;
        }

        $(ele).css("font-size", (originalFontSize * factor) + 'px');
        if (originalLineHeight) {
            $(ele).css("line-height", (originalLineHeight * factor) + 'px');
        }

    }
    $epubHtml.css("font-size", fontSize + "%");

};

Closing.

johanpoirier added a commit to TEA-ebook/readium-shared-js that referenced this issue Jan 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants