Created attachment 14022 [details] Testcase This testcase shows a div with a width of 100px. getComputedStyle reports its margin-right as the body's width minus 100, e.g. "906px".

The behavior is correct as far as I know. margin-left + width + margin-right = content width of containing block according to CSS2.1. Have you checked Firefox?

Firefox and Opera both return 0px. IE7 returns "auto". The current behavior makes it hard to do dynamic resizing, like "make the parent element just wide enough to contain its children".

also reported in Dojo: http://trac.dojotoolkit.org/ticket/3515

Marking confirmed per the above comments.

Also found / reported in Backbase

Incorrect behavior still happens. In my case: div (InnerDiv) placed into floated div with overflow hidden and fixed size. Inner div has paddings. In IE6/7, Opera 9 and Firefox getComputedStyle of InnerDiv for marginRight returns 0, but Safari returns negative number. I reasearch that problem: IE6/7, Opera 9 and Firefox works correct and calculate full width of InnerDiv as Width+marginRight+marginLeft but Safari calculate it as Width+marginRight+marginLeft+paddingRight+paddingLeft. So, bug is not fixed.

In our application, we render our pages off-screen before attaching them to the document because in a couple popular browsers the performance of off-screen (detached) DOM creation is an order of magnitude greater than on-screen (attached) DOM creation. Part of our rendering involves manually setting the width and heights on certain elements to fill the screen. (We do this manually for cross-browser compatibility.) If we can't know the margins of elements, then we can't size other elements appropriately. Please fix this bug! Thanks. Jon

Oops I replied to the wrong bug. Please ignore my comments here. Sorry!

Created attachment 23523 [details] Improved testcase I think the behavior in Safari is incorrect according to: http://www.w3.org/TR/CSS21/box.html#propdef-margin-right. It says the computed value of margin-right / left is the percentage as specified or the absolute length. So when i specify the margin, it should return that margin, the initial value (according to the specs) of the margin is 0. So by default it should return 0. I think the behavior in Safari is correct according to: http://www.w3.org/TR/CSS21/visudet.html#blockwidth "If 'width' is not 'auto' and 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + scrollbar width (if any) (plus any of 'margin-left' or 'margin-right' that are not 'auto') is larger than the width of the containing block, then any 'auto' values for 'margin-left' or 'margin-right' are, for the following rules, treated as zero. If all of the above have a computed value other than 'auto', the values are said to be "over-constrained" and one of the used values will have to be different from its computed value. If the 'direction' property of the containing block has the value 'ltr', the specified value of 'margin-right' is ignored and the value is calculated so as to make the equality true. If the value of 'direction' is 'rtl', this happens to 'margin-left' instead. "

Any news on this bug being fixed ?

Yes, get it fixed! There's a duplicate here: https://bugs.webkit.org/show_bug.cgi?id=13343 With a simple testcase attached

Created attachment 44533 [details] Very simple testcase This just shows an incorrect margin from computer style.

According Chromium bug report: http://code.google.com/p/chromium/issues/detail?id=23816

*** Bug 24511 has been marked as a duplicate of this bug. ***

This is STILL an issue. Any chance someone could take a look? A workaround is setting the element to display:none, measuring the margin then setting display back to block.

Temporarily setting it to inline-block works too. It's just a bodge though. As you say, can someone take a look?

Will be posting a proposed patch with additional/modified layout tests sometime today.

Created attachment 81218 [details] Proposed patch

Created attachment 81220 [details] CSS3 Version This patch is compliant with CSS3 for computed margin values. If "auto" is the supplied value, it will return "auto" as the computed value, per http://www.w3.org/TR/css3-box/#the-margin. We can have a popular vote as to which patch (this or the previous) should be the fix.

@Jarred: nice. Does the test case "crbug_23816.html" (see comment #14) pass?

(In reply to comment #21) > @Jarred: nice. Does the test case "crbug_23816.html" (see comment #14) pass? Yes it does. I am summing up my opinions on some compliance issues/inconsistencies...expect a pretty lengthy follow up comment shortly :)

Bare with me, this will be a lengthy comment, but I just wanted to weigh in my opinion on this bug. My feeling is that the original behavior is the correct one, OR the current behavior of width/height (among others) is wrong. We can use CSS 2.1 (http://www.w3.org/TR/CSS21/visudet.html#the-width-property & http://www.w3.org/TR/CSS21/box.html#margin-properties) or CSS 3 (http://www.w3.org/TR/css3-box/) as our guide. Margin's computed values are said to be this: "the percentage as specified or the absolute length or ‘auto’". Some have understood "absolute length" to be what was specified (including Firefox, Opera, IE, and everyone CC'ed on this bug). The original behavior understood "absolute length" to mean "the used value". Now if we take width/height's computed value definition: "the percentage or ‘auto’ as specified or the absolute length; ‘auto’ if the property does not apply". You'll notice that "absolute length" is again used, when a percentage or auto isn't specified. The difference here is that all browsers, including WebKit, always return "the used value" as the computed width. So if we specified "auto", "50%", or "500px", we always get the used value in fixed pixels back. If our container block is 1000px wide, the computed value for those 3 cases would be "1000px", "500px", and "500px" respectively. So currently, margin-left: auto; and margin-right: auto; will return a computed value of the "used value" in fixed pixels. If this isn't right, then width/height aren't right either. So the question is, how do we define "absolute length". Is it the absolute fixed length specified, or is it the used value? It seems like the world has come to a conclusion that it's the specified value. In the case margin is not specified, an initial value of 0 is used. In the case of width/height though, it would appear that no browser is compliant. I would expect width: auto; to return "auto", width: 50%; to return "50%", and width: 500px; to return "500px", and an undefined width to return "auto". The CSS3 patch I've attached to this bug will make margin's compliant to CSS3, but it seems width/height (and several others) need fixing too. So what does everyone think? Do we go full compliant, or do we just try to act like the rest of the browsers? Thanks for reading my rant :)

If the specs are unclear, you should raise the issue on the www-style mailing list.

(In reply to comment #24) > If the specs are unclear, you should raise the issue on the www-style mailing list. Sure, my apologies. Nevertheless, I think the first "Proposed patch" is the one to go with, as it will match every other major browser out there, and match CSS 2.1 for margins. It should be ready to review.

Comment on attachment 81218 [details] Proposed patch Can you get rid of the extraneous lines... there's an extra blank line in every case statement now.

Created attachment 82023 [details] Proposed patch removed blank lines, updated changelog date

Comment on attachment 82023 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=82023&action=review > LayoutTests/fast/css/getComputedStyle/getComputedStyle-margin-auto.html:45 > + document.write("width: " + window.getComputedStyle(foodiv, null).getPropertyValue("width")); > + document.write( "<br>"); > + document.write("height: " + window.getComputedStyle(foodiv, null).getPropertyValue("height")); > + document.write( "<br>"); > + document.write("margin-left: " + window.getComputedStyle(foodiv, null).getPropertyValue("margin-left")); > + document.write( "<br>"); > + document.write("margin-right: " + window.getComputedStyle(foodiv, null).getPropertyValue("margin-right")); > + document.write( "<br>"); > + document.write("margin-top: " + window.getComputedStyle(foodiv, null).getPropertyValue("margin-top")); > + document.write( "<br>"); > + document.write("margin-bottom: " + window.getComputedStyle(foodiv, null).getPropertyValue("margin-bottom")); > + document.write( "<br>"); > + document.write("padding-left: " + window.getComputedStyle(foodiv, null).getPropertyValue("padding-left")); > + document.write( "<br>"); > + document.write("left: " + window.getComputedStyle(foodiv, null).getPropertyValue("left")); > + document.write( "<br>"); Please look at how script tests are done (/Tools/Scripts/make-script-test-wrappers and examples in fast/css/script-tests). document.write is evil.

(In reply to comment #28) > document.write is evil. Will update, I adapted the existing test. I should have updated it.

Created attachment 82162 [details] Proposed patch improved tests

Comment on attachment 82162 [details] Proposed patch Rejecting attachment 82162 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'bu..." exit_code: 2 Last 500 characters of output: ................................................................................................................................................. fast/css/content .. fast/css/counters ....................... fast/css/getComputedStyle ....... fast/css/getComputedStyle/computed-style-without-renderer.html -> failed Exiting early after 1 failures. 6786 tests run. 150.97s total testing time 6785 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 3 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/7878044

This must be a very recently added test.  I haven't looked at it yet but it's probably compensating for the old behavior.  Will take a look when at a machine.

Created attachment 82248 [details] Proposed patch Update expected margin values on computed-style-without-renderer-expected.txt for all platforms.

I agree with the proposed patch. Having getComputedStyle return the size of the gap between the element's edge and the associated edge of its container is just not a *useful* operation. Intuitively, getComputedStyle is used for finding out the end result of any CSS rules applied to the element at that time. You are asking what 'margin-right' style has been arrived at by evaluating all matched style rules.

Comment on attachment 82248 [details] Proposed patch Clearing flags on attachment: 82248 Committed r78436: <http://trac.webkit.org/changeset/78436>

All reviewed patches have been landed. Closing bug.

This is now against how IE9, FF3.6+, and Opera implementation of getComputedStyle() for margins. Open up the following jsfiddle in FF3.6+, Opera, and IE9: http://jsfiddle.net/u4F8m/9/ It is also against the CSSOM editor's draft: http://dev.w3.org/csswg/cssom/#resolved-value http://www.w3.org/TR/css3-values/ It also now makes margin inconsistent with how padding, width, and height work for resolved values in Webkit itself. I would also argue that "Having getComputedStyle return the size of the gap between the element's edge and the associated edge of its container is just not a *useful* operation." is not true. It's very useful to now the exact pixel dimensions of a box. Why would I ever want to know that a margin is "10%"? 10% of what? The actual pixel value used is way more relevant. Please consider reopening and reverting this change.

> Please consider reopening and reverting this change. Could you please open a new bug? Your comment makes sense and needs to be investigated. However if posted on a closed bug, it is lost for WebKit developers as it won't show up on most bugzilla queries. Thanks!

new ticket: https://bugs.webkit.org/show_bug.cgi?id=73334