Closed
Bug 393910
Opened 18 years ago
Closed 13 years ago
Serialize "0" as "0px" for lengths instead of "0pt"
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: jruderman, Assigned: ayg)
References
Details
Attachments
(2 files, 2 obsolete files)
327 bytes,
text/html
|
Details | |
3.82 KB,
patch
|
ayg
:
review+
|
Details | Diff | Splinter Review |
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.
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.
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.
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.)
Updated•16 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 5•13 years ago
|
||
Test-case:
data:text/html,<!doctype html>
<script>
document.head.style.height = "0";
document.documentElement.textContent = document.head.style.height
</script>
IE10 Developer Preview: 0px
mozilla-central: 0pt
Chrome 18 dev: 0px
Opera Next 12.00 alpha: 0px
Spec:
"A length of zero is represented by the literal string "0px"."
http://dev.w3.org/csswg/cssom/#serialize-a-css-value-component
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.
Assignee | ||
Comment 6•13 years ago
|
||
This serializes it as "0px", like the spec and other browsers.
Assignee: nobody → ayg
Attachment #596762 -
Flags: review?(dbaron)
Assignee | ||
Updated•13 years ago
|
Whiteboard: [autoland]
Updated•13 years ago
|
Whiteboard: [autoland] → [autoland-in-queue]
Comment 7•13 years ago
|
||
Autoland Patchset:
Patches: 596762
Branch: mozilla-central => try
Destination: http://hg.mozilla.org/try/pushloghtml?changeset=56185fcd57d1
Try run started, revision 56185fcd57d1. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=56185fcd57d1
Comment on attachment 596762 [details] [diff] [review]
Patch v1
>+// Bug 393910
>+p.setAttribute("style", "margin-left: 0");
>+is(p.style.getPropertyValue("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
Attachment #596762 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 9•13 years ago
|
||
Attachment #596762 -
Attachment is obsolete: true
Attachment #596770 -
Flags: review?(dbaron)
Assignee | ||
Updated•13 years ago
|
Attachment #596770 -
Flags: review?(dbaron) → review+
Comment 10•13 years ago
|
||
Try run for 56185fcd57d1 is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=56185fcd57d1
Results (out of 210 total builds):
exception: 3
success: 164
warnings: 29
failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-56185fcd57d1
Updated•13 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Comment 11•13 years ago
|
||
There was a test I missed in content/base/, because I only tested layout/style/ locally.
Attachment #596770 -
Attachment is obsolete: true
Attachment #597038 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Keywords: testcase → checkin-needed
Summary: Output unitless zero for style.width ("0" instead of "0pt") → Serialize "0" as "0px" for lengths instead of "0pt"
Comment 12•13 years ago
|
||
Comment 13•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•