getComputedStyle() returns unitless values for some properties that require units

• Component: CSS
• Status: RESOLVED FIXED

[Rob Brackett]

Created attachment 83598 [details]
Test case for unit-less properties in getComputedStyle()

The values returned by getComputedStyle() for some properties that require units do not include units. For example:

<div id="test" style="-webkit-column-gap: 20px"></div>
<script type="application/javascript">
  getComputedStyle(document.getElementById("test")).webkitColumnGap; // "20" (should be "20px")
</script>

The properties exhibiting the issue are:
-webkit-column-width
-webkit-column-gap
-webkit-perspective

A test case is attached. Properties in red do not include units. At current, there are only three buggy properties and they are equally buggy when display is "none" as when it is not, but experience with getComputedStyle() bugs has taught me the value of a comprehensive test ;)

[Emil A Eklund]

Created attachment 90103 [details]
Patch

[Ojan Vafai]

Comment on attachment 90103 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90103&action=review

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-length-unit.html:93
> +        for (var propertyName in propertiesToCheck) {
> +            testElement.style[propertyName] = propertiesToCheck[propertyName];
> +        }
> +        
> +        function testPropertyUnit(className, propertyName)
> +        {
> +            if (testElement.className != className)
> +                testElement.className = className;
> +            var computedStyled = document.defaultView.getComputedStyle(testElement);
> +            var results = /\d+([a-z%]+)/.exec(computedStyled[propertyName]);
> +            return results ? results[1] : '';
> +        }
> +
> +        for (var propertyName in propertiesToCheck) {
> +            shouldBe("testPropertyUnit('visible', '" + propertyName + "')", "'px'");
> +        }
> +        for (var propertyName in propertiesToCheck) {
> +            shouldBe("testPropertyUnit('hidden', '" + propertyName + "')", "'px'");
> +        }
> +
> +        for (var propertyName in propertiesToCheck) {
> +            testElement.style[propertyName] = '';
> +        }

nit: although we don't enforce style for tests, webkit style is to not include curly braces for one-line if/else/for/while statements.

[Emil A Eklund]

Created attachment 91719 [details]
Patch

Thanks Ojan. Fixed the style issues in the layout test, please take another look.

[WebKit Commit Bot]

Comment on attachment 91719 [details]
Patch

Clearing flags on attachment: 91719

Committed r85418: <http://trac.webkit.org/changeset/85418>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.