Closed Bug 1431031 Opened 6 years ago Closed 6 years ago

calc() css function doesn't add 2 values expressed in 'rem' and 'em' units, when on <html/> root element

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Version:
57 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: rjezyk, Assigned: emilio)

Details

Attachments

(3 files)

firefox-screenshot-px.png
236.24 KB, image/png
Details
firefox-screenshot-rem.jpg
337.77 KB, image/jpeg
Details
Bug 1431031: Make the font-size calc() code do what it means to do.
59 bytes, text/x-review-board-request
manishearth
: review+
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

I applied calc() function with complex value, to the root <html/> element:

html { font-size: calc(1rem + (1.25 - 1) * (100vw - 20em) / (75 - 20)); }

to achieve responsive, fluid typography on my website.


Actual results:

Addition of 1rem from inside of the body of the calc() function get omitted.
So instead of ie. having 1rem + 5px, with default browser font-size is 16px, should compute the font size to 21px. Instead, it computed it to 5px, ignoring 1rem value completely. Changing aforementioned CSS rule to:

html { font-size: calc(16px + (1.25 - 1) * (100vw - 20em) / (75 - 20)); }

or even:

html { font-size: calc(1em + (1.25 - 1) * (100vw - 20em) / (75 - 20)); }

works as expected, and in line with the rest of the browsers.

I tested it in Firefox version 57.0.4 and in Firefox Developer Edition version 58.0b16


Expected results:

1rem value from the function body:

html { font-size: calc(1rem + (1.25 - 1) * (100vw - 20em) / (75 - 20)); }

should be added to the rest of computation

Further (simplified test ie. font-size: calc(1rem + 1em)), confirmed this behaviour. What's significant here is I think the fact that it is happening only in the context of <html/> root element
Flags: needinfo?(emilio)
I don't see what the bug is. For me, both in 57 and in Nightly:

  data:text/html,<style>html { font-size: calc(1rem + 1em) };</style>Which font size am I?

Renders the same as:

  data:text/html,<style>html { font-size: calc(16px + 1em) };</style>Which font size am I?

As expected, because rem and em units on the root element are resolved relatively to the initial font-size.

Other browsers also agree on this.

Could you provide a test-case (as an attachment), and maybe a screenshot or a reference of how that should render? Thanks!
Flags: needinfo?(emilio) → needinfo?(rjezyk)
html { font-size: calc(16px + 1em) } computes to 32px, as I would expect
Flags: needinfo?(rjezyk)
html { font-size: calc(1rem + 1em) } computes (surprisingly) to 16px
You can test it out directly in Element Inspector. I used Mozilla home page.

In fact, using calc() function the way I describe (when adding rem values to em values), I can do crazy stuff like font-size: calc(19999999999999999rem + 1em) and this will always compute to 16px

If you need a live example I can (but prefer not to) bring back the live version of the website I had the bug on. But obviously, I would like to avoid that.
Ahá, so this is because the root has a lang="en" attribute... Here's a test-case that fails:

  data:text/html,<html lang="en"><style>html { font-size: calc(1rem + 1em); };</style>Which fontsize am I?</html>

I hate our font-size + lang stuff :(

Thank you very much!
Status: UNCONFIRMED → NEW
Ever confirmed: true
No problem at all. I used workaround and used 'ems' instead of 'rem' in my calculation :D
But it is so interesting, how these two: calc() and attribute lang="" relate to each other? 
I'll need to do some reading on 'lang' attribute then...;) Can it be used on other HTML elements too? (<div lang="en"/>)
Will you bother to give some explanation on the nature of the bug itself?
font-size has some tricky interactions with the language and font family: https://manishearth.github.io/blog/2017/08/10/font-size-an-unexpectedly-complex-css-property/

We probably don't set the correct default value for the keyword ratio stuff.
We should just kill that code and make the font not depend on the language. And remove MathML and his font-size woes, too.
Assignee: nobody → emilio
Comment on attachment 8943311 [details]
Bug 1431031: Make the font-size calc() code do what it means to do.

Blerg, this is still busted for ex / ch / etc...
Attachment #8943311 - Flags: review?(manishearth)
Comment on attachment 8943311 [details]
Bug 1431031: Make the font-size calc() code do what it means to do.

https://reviewboard.mozilla.org/r/213620/#review219576

::: servo/components/style/values/specified/length.rs:146
(Diff revision 3)
>                                  reference_font_size.into()
>                              );
>                      }
>                  }
> +
> +                if matches!(base_size, FontBaseSize::InheritedStyleButStripEmUnits) {

nit: use an if let or if with an `==`
Attachment #8943311 - Flags: review?(manishearth) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a984a3218ffe
Tests for calc() on font-size on various situations. r=Manishearth
Actual fix in https://hg.mozilla.org/mozilla-central/rev/110b50e49ca4.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: