Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 393910 - Serialize "0" as "0px" for lengths instead of "0pt"
: Serialize "0" as "0px" for lengths instead of "0pt"
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla13
Assigned To: Aryeh Gregor (:ayg) (away until October 25)
: Jet Villegas (:jet)
: 516442 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2007-08-27 15:20 PDT by Jesse Ruderman
Modified: 2012-02-16 02:50 PST (History)
5 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (327 bytes, text/html)
2007-08-27 15:20 PDT, Jesse Ruderman
no flags Details
Patch v1 (3.31 KB, patch)
2012-02-13 12:57 PST, Aryeh Gregor (:ayg) (away until October 25)
dbaron: review+
Details | Diff | Splinter Review
Patch v2 (3.17 KB, patch)
2012-02-13 13:14 PST, Aryeh Gregor (:ayg) (away until October 25)
ayg: review+
Details | Diff | Splinter Review
Patch v3, test failure fixed (3.82 KB, patch)
2012-02-14 08:20 PST, Aryeh Gregor (:ayg) (away until October 25)
ayg: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2007-08-27 15:20:31 PDT
Created attachment 278463 [details]

It would be nice if "width: 0" would stay as "0" instead of becoming "0pt" when serialized.  I rarely use pt units so it's strange to see them appear unnecessarily.
Comment 1 David Baron :dbaron: ⌚️UTC-7 2007-08-27 18:26:44 PDT
IE7/Windows gives 0px
Opera 9.23/Linux gives 0
Konqueror gives 0px

I agree that 0 is better style; not sure if I want to make the switch at this point in the cycle.  Nobody else gives pt, though.
Comment 2 David Baron :dbaron: ⌚️UTC-7 2007-08-27 18:32:01 PDT
We could make a simple fix in nsCSSDeclaration::AppendCSSValueToString , but it would be nice to round-trip the unit as specified.  We currently store 0s as various default units in CSSParserImpl::TranslateDimension; the way to round-trip properly would involve adding a new unit for them and making sure that unit was handled appropriately.
Comment 3 David Baron :dbaron: ⌚️UTC-7 2007-08-27 18:36:15 PDT
I think this bug should really be about round-tripping what the author specified rather than converting other values to 0, though.

(The reason for points as the default in TranslateDimension may have been that points were cheaper to compute back when we used twips; that's no longer the case following bug 177805.)
Comment 4 Dave Garrett 2009-09-14 09:55:53 PDT
*** Bug 516442 has been marked as a duplicate of this bug. ***
Comment 5 Aryeh Gregor (:ayg) (away until October 25) 2012-02-13 12:39:18 PST

data:text/html,<!doctype html>
<script> = "0";
document.documentElement.textContent =

IE10 Developer Preview: 0px
mozilla-central:        0pt
Chrome 18 dev:          0px
Opera Next 12.00 alpha: 0px


"A length of zero is represented by the literal string "0px"."

Seems like "0px" is the winner here.  "0" makes more sense, but nobody does that right now, so it makes more sense to go with what everyone else does.
Comment 6 Aryeh Gregor (:ayg) (away until October 25) 2012-02-13 12:57:39 PST
Created attachment 596762 [details] [diff] [review]
Patch v1

This serializes it as "0px", like the spec and other browsers.
Comment 7 Mozilla RelEng Bot 2012-02-13 13:01:35 PST
Autoland Patchset:
	Patches: 596762
	Branch: mozilla-central => try
Try run started, revision 56185fcd57d1. To cancel or monitor the job, see:
Comment 8 David Baron :dbaron: ⌚️UTC-7 2012-02-13 13:06:46 PST
Comment on attachment 596762 [details] [diff] [review]
Patch v1

>+// Bug 393910
>+p.setAttribute("style", "margin-left: 0");
>+is("margin-left"), "0px",
>+  "0 serializes to 0px");
> for (var test in tests) {

Could you put your insertion after the main for loop instead of before?

r=dbaron with that
Comment 9 Aryeh Gregor (:ayg) (away until October 25) 2012-02-13 13:14:29 PST
Created attachment 596770 [details] [diff] [review]
Patch v2
Comment 10 Mozilla RelEng Bot 2012-02-13 23:15:58 PST
Try run for 56185fcd57d1 is complete.
Detailed breakdown of the results available here:
Results (out of 210 total builds):
    exception: 3
    success: 164
    warnings: 29
    failure: 14
Builds (or logs if builds failed) available at:
Comment 11 Aryeh Gregor (:ayg) (away until October 25) 2012-02-14 08:20:28 PST
Created attachment 597038 [details] [diff] [review]
Patch v3, test failure fixed

There was a test I missed in content/base/, because I only tested layout/style/ locally.
Comment 13 Marco Bonardo [::mak] 2012-02-16 02:50:31 PST

Note You need to log in before you can comment on or make changes to this bug.