New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 229280 link

Starred by 16 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 150741
issue 176671



Sign in to add a comment

getComputedStyle returns percentages for left / right / top / bottom

Project Member Reported by tansell@chromium.org, Apr 9 2013

Issue description

Migrated from WebKit Bugzilla: https://bugs.webkit.org/show_bug.cgi?id=29084
Originally reported 2009-09-09 07:07 PST by Jake Archibald (jaffathecake@gmail.com).


Description:
var div = document.createElement('div');
div.style.position = 'absolute';
div.style.top = '10%';
document.body.appendChild(div);
alert( window.getComputedStyle(div, null).top );

The above will alert '10%' rather than the pixel value.

Gecko / Presto will give the pixel value.

Webkit gives the pixel value for other properties (widht, height etc) and will give the pixel value for left / right / top / bottom if the set value isn't a percent, eg 10em.

The above bug exists in Safari, Chrome and Nightly r48199, although I have only tested on Windows.

Jake.


Blocks:
[http://wkb.ug/106632] NEW - getComputedStyle on animation from pixels to percent returns null or asserts
[http://wkb.ug/110007] NEW - [meta] Issues that jQuery is working around.

Attachments:
2012-07-10 14:17 PST: test [https://bugs.webkit.org/attachment.cgi?id=151525&action=prettypatch]
2012-07-16 14:37 PST: work in progress [https://bugs.webkit.org/attachment.cgi?id=152616&action=prettypatch]
2013-02-12 00:33 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=187794&action=prettypatch]
2013-02-13 01:01 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=188032&action=prettypatch]
2013-02-13 18:01 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=188242&action=prettypatch]
2013-02-14 21:03 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=188473&action=prettypatch]
2013-02-17 17:58 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=188785&action=prettypatch]
2013-02-18 04:19 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=188845&action=prettypatch]
2013-02-21 01:27 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=189471&action=prettypatch]
2013-02-21 02:21 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=189487&action=prettypatch]
2013-02-21 02:45 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=189490&action=prettypatch]
2013-02-21 20:07 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=189665&action=prettypatch]
2013-02-25 18:05 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=190170&action=prettypatch]



Comments:
================================

<snip>

================================

Comment #96
Posted on 2013-03-08 13:17:27 PST by Eric Seidel (eric@webkit.org)

(From update of attachment 190170 [https://bugs.webkit.org/attachment.cgi?id=190170] [details] [https://bugs.webkit.org/attachment.cgi?id=190170&action=edit])
View in context: https://bugs.webkit.org/attachment.cgi?id=190170&action=review

OK.  This change looks great.  My only concern is about testing.  There are a lot of cases here.  I'm not sure you covered position: sticky.  You shoudl be aware of the many types of lenghts:
enum LengthType {
    Auto, Relative, Percent, Fixed,
    Intrinsic, MinIntrinsic,
    MinContent, MaxContent, FillAvailable, FitContent,
    Calculated,
    ViewportPercentageWidth, ViewportPercentageHeight, ViewportPercentageMin, ViewportPercentageMax,
    Undefined
};

You shouldn't need to care about those, but its good to knwo it exists.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:641
> +    // See http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSview-getComputedStyle

Maybe you meant this?
http://dev.w3.org/csswg/cssom/#widl-Window-getComputedStyle-CSSStyleDeclaration-Element-elt

If the property applies to a positioned element and the resolved value of the 'display' property is not none, the resolved value is the used value. Otherwise the resolved value is the computed value.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:645
> +            return zoomAdjustedPixelValueForLength(style->left(), style);

I might have moved these switch statements into helper functions.  Then this code looks like:

if (!renderView || !renderer || !renderer->isBox())
    return zoomAdjustedPixelValueForLength(lengthForProperty(propertyId), style);

One could do similar for the other switches, presumably:
relativePositionForProperty(propertyId, box);
and
positionForProperty(propertyId, box, containingBlock);

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:-661
> -        else if (l.isViewportPercentage())

Do we have test coverage for this behavior?  You appear to be removing it in the display: none case.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:659
> +    if (box->isRelPositioned() || !containingBlock) {

I don't believe we can hit this case?  You will always have a containing block if you're in the DOM tree, I think?  Unless we're talking about <html style='position: relative'>?  If the element isn't in the tree, it can't have a renderer.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:687
> +    return 0;

We need tests requesting top/right/bottom/left on elements which are not positioned.

================================

Comment #97
Posted on 2013-03-08 13:20:40 PST by Eric Seidel (eric@webkit.org)

(From update of attachment 190170 [https://bugs.webkit.org/attachment.cgi?id=190170] [details] [https://bugs.webkit.org/attachment.cgi?id=190170&action=edit])
View in context: https://bugs.webkit.org/attachment.cgi?id=190170&action=review

> Source/WebCore/ChangeLog:13
> +        This is now consistent with both release Firefox and IE9.

You might add more links/documetnation here.  Your ChangeLog is sorta your "advertisement" for your change.  Tells all the "whys" and how we'd be fools to not accept your amazing change. :)


 
Cc: mit...@mithis.com
Cc: sandholm@chromium.org
 Issue 44266  has been merged into this issue.
Blocking: chromium:150741

Comment 4 by m.go...@gmail.com, Apr 9 2013

This should block  issue 176671 .
Cc: tansell@chromium.org pfeldman@chromium.org
 Issue 228938  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, May 20 2013

Labels: -WebKit-ID-29084 WebKit-ID-29084-ASSIGNED
https://bugs.webkit.org/show_bug.cgi?id=29084

Has there been any movement on this since the switch to Blink for Chrome? The WebKit patch was almost there. 

Comment 8 by mit...@gmail.com, Jul 24 2013

The patch was moved to Blink and uploaded to https://codereview.chromium.org/13871003/ it almost got committed before it hit a snag and I never got back to it.



Comment 9 by mkwst@chromium.org, Jul 28 2013

Blocking: chromium:176671

Comment 10 by vli@chromium.org, Nov 19 2013

Labels: Hotlist-DevRel
Status: Assigned
This is still an issue for "auto", although it seems like the percent -> pixel issue has been resolved:

http://jsfiddle.net/37wdV/

Comment 13 by tkent@chromium.org, Jun 24 2014

Labels: -WebKit-CSS Cr-Blink-CSS Hotlist-Fixit
Labels: -Hotlist-Fixit Hotlist-Interop
Cc: -sandholm@chromium.org
Status: Fixed
This appears to be fixed in Chrome stable 45: http://jsfiddle.net/66msxeqb/
Isn't there still an issue with auto? (comment #12)
Owner: ----
Status: Available
Summary: getComputedStyle returns "auto" for left / right / top / bottom (was: getComputedStyle returns percentage values for left / right / top / bottom)
Test case for auto: http://jsfiddle.net/xu5b7rLq/

Chrome 46 outputs:
The following should all be pixel values:
auto auto auto auto

Firefox 42 ouputs:
The following should all be pixel values:
27px 8px 275px 669px

Comment 19 by mit...@mithis.com, Feb 24 2016

Cc: -mit...@mithis.com
Status: Fixed (was: Available)
Summary: getComputedStyle returns percentages for left / right / top / bottom (was: getComputedStyle returns "auto" for left / right / top / bottom)
The original report has actually been fixed, I moved the issue with auto into a separate bug, 589347.

Sign in to add a comment