1348169 - Fix NaN shown in the Box Model's margins

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[magicp]

Attachment: nan-in-boxmodel.png

Steps to reproduce:
1. Start latest Nightly
2. Go to about:home
3. Open Inspector > Computed
4. Select #downloads:before in markup
5. See Box Model

Actual Results:
"NaN" shown for margin-left/margin-right in the Box Model.

Expected Results:
"auto" should be shown for margin-left/margin-right in the Box Model.

Another step to reproduce:
In step 4, select #searchSuggestionTable. "NaN" shown for position-top/position-right/position-bottom.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fcef571d22c3ad151a62cb1cad6d751f79a72446&tochange=fd85a5aafa9fdc2fbe012408e996f573308fb697

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1348169 - Fix NaN shown in the Box Model's margins.

Review commit: https://reviewboard.mozilla.org/r/121938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121938/

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

https://reviewboard.mozilla.org/r/121938/#review123940

This looks like the kind of things that would really benefit from a new non-reg test.
We know exactly what makes it fail, it would be very easy to add a new test that prevents it in the future.
Can you add one please?

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/121938/diff/1-2/

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/121938/diff/2-3/

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

https://reviewboard.mozilla.org/r/121938/#review126726

Thanks for adding this test. It looks nice.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48848888a7b3
Fix NaN shown in the Box Model's margins. r=pbro

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/efb00df6a299
Fix NaN shown in the Box Model's margins. Fix eslint issues. r=style-fix on a CLOSED TREE

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/48848888a7b3
https://hg.mozilla.org/mozilla-central/rev/efb00df6a299

Comment 9

[roxleitan]

Verified fixed using the latest Nightly 55.0a1 (2017-03-31) on Ubuntu 16.04, Mac OS X 10.12 and Windows 10 x64. "Auto" appears in the Box Model for margin-left/margin-right.

Comment 10

[magicp]

The latest Developer Edition (20170331004006) is affected. Please uplift to 54.

Comment 11

[Julien Cristau [:jcristau]]

(In reply to magicp from comment #10)
> The latest Developer Edition (20170331004006) is affected. Please uplift to
> 54.

Gabriel, what do you think about uplift to beta at this point?

Comment 12

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

Approval Request Comment
[Feature/Bug causing the regression]: 1336198
[User impact if declined]: NaN shown in the Box Model's margins.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Already verified.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Covered by unit tests and a simple number parsed error.
[String changes made/needed]: None

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8849108 [details]
Bug 1348169 - Fix NaN shown in the Box Model's margins.

We've already built RC build for 54 today and this is too late for 54. Beta54- and mark 54 won't fix.

Comment 14

[Nazir Ahmed Sabbir [:NaSb]]

I reproduced this bug mentioned in comment 0 using Nightly 55.0a1 (2017-03-16) (64-bit),Build ID: 20170316110253 on Linux x64.

This Bug is now verified as fixed on Latest Firefox Nightly 55.0a1 (2017-06-06) (64-bit)

Build ID: 20170606100219
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
OS: Linux 4.8.0-53-generic

Comment 15

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 55.0a1 (2017-03-16) (64-bit) on Windows 7, 64 Bit!

This bug's fix is verified with latest Nightly!

Build ID   :  20170606030207
User Agent :  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170607]

Comment 16

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce the issue mentioned in comment 0 using Firefox 55.0a1 (BuildId:20170316030211).

This issue is verified fixed on Firefox 55.0b3 (BuildId:20170619071723) using Windows 10 64bit, Ubuntu 16.04 64bit and macOS 10.11.6.