989802 - rounding of viewport units sometimes rounds up, leading to things expected to fit not fitting in a container

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Marek Raida]

Attachment: bug.htm

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140330030202

Steps to reproduce:

Basic HTML page with layout using box-sizing: border-box, display: inline-block and vw units.

The only remotely (perhaps) related and open issues to this I was able to find are: 176237 and 935850


Actual results:

If you try to resize window with four boxes side-by-side to several window's widths, sometimes, the last box in line drops to the beginning of next floating row, which is not wanted and not logical at all; it happens probably because of some computation/math rounding related problem. For me, from approx. 20 tested width sizes, some fit 13 and some 7 not.
Discovered by my colleague Robin and demonstrated here http://codepen.io/robinpokorny/full/iFemu + I'm enclosing reduced testcase as attachment to this issue as well. BTW changing anything in .wrapper class fixes/affects the broken behavior.

It is broken in both current stable Firefox release and latest nightly build.


Expected results:

All four boxes should stay in one line on all occasions.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think I see the underlying bug, which is specific to viewport units and not to box-sizing.

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Untested work-in-progress patch:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/2a40e7a0a2b0/round-viewport-units

I'll test this (and write an automated test) later; have other things to do on a nice Sunday afternoon.

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(Though technically I think what I want is round-towards-zero rather than floor; that'll be a little more trouble since we don't have a method for that already.)

Comment 4

[Marek Raida]

Thx David a lot for diving into the issue so quickly, we appreciate it a lot...

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Sorry for the delay getting back to this; finally got a chance to finish it up this morning.


Try push: https://tbpl.mozilla.org/?tree=Try&rev=c7716cca08da

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: Round viewport units to appunits using trunc rather than round so that repeated uses fit within a container

viewport-units-rounding-1.html fails without the patch and passes with
the patch.

viewport-units-rounding-2.html fails with an early version of the patch
but not without the patch or with the final version.

Comment 7

[Marek Raida]

Oh, do not apologize for delay, it is still great that you've reprioritized and get to this.
Thank you...

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

From the try run, it looks like I need to fix two tests in layout/reftests/css-valuesandunits/.

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

https://tbpl.mozilla.org/?tree=Try&rev=7670542545c9

Comment 10

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: Bug 989802 - Round viewport units to appunits using trunc rather than round so that repeated uses fit within a container.

There turned out not to be a reasonable way to fix he test, but fixing the code to divide by 100.0f (which can be exactly represented as a float) rather than multiplying (earlier) by 0.01f (which can't be represented exactly) fixes the test failures, so I'm inclined to do just that.

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/68736e0362c3

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/68736e0362c3