Closed Bug 381328 Opened 13 years ago Closed 2 years ago

getComputedStyle returns incorrect results for used margin-right/left when 'auto' or overconstrained

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1298008

People

(Reporter: sharparrow1, Unassigned, Mentored)

References

()

Details

(Keywords: testcase, Whiteboard: [firebug-p3])

Attachments

(1 file)

Essentially, the issue is with the formula at http://www.w3.org/TR/CSS21/visudet.html#blockwidth

(Abridged text copied for convenience.)

  'margin-left' + 'border-left-width' + 'padding-left' + 'width' +
  'padding-right' + 'border-right-width' + 'margin-right' +
  scrollbar width (if any) = width of containing block

  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.

The most basic test is the following:
<div style="width: 500px">
<div id=elem style="width: 600px; margin: 0; padding: 0; border: 0">
Test
</div>

According to the rules, the specified margin-right value should be ignored and the used value computed from the formula; this gives a used margin-right of -100px.  However, getComputedStyle returns 0px.
Hmm, it looks like the bug is mostly in nsHTMLReflowState: we're storing the used margin into the frame property much too early, before we apply the formula from 10.3.3.  It's only visible through getComputedStyle because we don't use the results of the margin-right of GetUsedMargin anywhere else.

There's also the issue that we assume in GetUsedMargin that if all the margins are absolute lengths, they're all the same as the used values.
Severity: normal → minor
See also Bug 258255 and 356665.  This bug makes workaround for those bugs more difficult. Maybe the have the same root cause.
Confirmed with current trunk and FB1.2r389. Firebug's "layout" box, which I assume is using getComputedStyle, shows margins of 0 on the parent div. [Simple testcase from comment 0]
Severity: minor → normal
Whiteboard: [firebug-p1]
Version: unspecified → Trunk
Whiteboard: [firebug-p1] → [firebug-p3]
Keywords: testcase
OS: Windows XP → All
Hardware: PC → All
(In reply to comment #0)
> Essentially, the issue is with the formula at
> http://www.w3.org/TR/CSS21/visudet.html#blockwidth

Watch out! getComputedStyle() should comply to the CSS2 (not CSS2.1) spec

http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSview-getComputedStyle
  This method is used to get the computed style as it is defined in [CSS2].

http://www.w3.org/TR/CSS2/visudet.html#q6
  [...]and one of the COMPUTED values will have to be different from its SPECIFIED value.

Thus, the COMPUTED value of margin-right should be -100px

What Firefox actually does is comply to CSS2.1:
http://www.w3.org/TR/CSS21/visudet.html#blockwidth
  The following constraints must hold among the USED values of the other properties:
[...]one of the USED values will have to be different from its COMPUTED value.

This means, computed value = 0px (which is wrong); used value = -100px

The concept of "used values" has not been established in CSS2.
(cf. http://www.w3.org/TR/CSS21/changes.html#q35 )
(In reply to comment #4)
> This means, computed value = 0px (which is wrong); used value = -100px

Let me clarify: computed value = 0px is actually right (CSS2.1) but should not be returned by getComputedStyle().
this behavior based on the testcase from c#0 is still true in Firefox 3.6b. If T.Rosenau is correct and we're conforming to CSS2.1, is there anything we need to do here? Safe to close this?
Blocks: 610966
0px is also returned when margin is set to auto and the value is not overconstrained: http://jsfiddle.net/RVpPx/1/
From the mdn docs for used values https://developer.mozilla.org/en/CSS/used_value it should be returning ((width of containing block) - 'width')/2 instead of 0px
This change is being made in webkit as we speak: https://bugs.webkit.org/show_bug.cgi?id=73334

To summarize: http://www.w3.org/TR/css3-values/#stages-examples says that when auto: is specified, the used value should be the positive pixel value, not 0px. It's also clarified again in the CSSOM draft here: http://dev.w3.org/csswg/cssom/#resolved-value

If we are to be following CSS3, Webkit, and IE9/10, the conclusion is: when margin is "auto", getComputedStyle should return the actual used pixel value.
The change has now been made and is live in Webkit and Trident. 
In the meantime, it's been a continuing thorn in jQuery's side. I've had several bug reports about this, and the most recent is:
http://bugs.jquery.com/ticket/11813

Of course, you guys shouldn't do work just to lower jQuery's bug count, but I'm just trying to provide context as to how this effects real world usage.
We need to test this for all values of 'position'.
Summary: getComputedStyle returns incorrect results for used margin-right/left → getComputedStyle returns incorrect results for used margin-right/left when 'auto' or overconstrained
Here's a canonical example of this issue:
http://jsfiddle.net/mpetrovich/yvj9b/

In FF, getComputedStyle() incorrectly reports the left margin as 0px instead of 99px in Webkit/IE.
In the Firefox devtools, we want to show the padding and margin regions of a node (see attachment 538850 [details] for a screenshot in Chrome), but this bug is blocking us.

If fixing this bug is not too hard, maybe I can help if I'm mentored.
Blocks: 663778
No longer blocks: 663778
FWIW, this now works in IE>8, Chrome, Opera, and Safari. FF is the only remaining browser that returns zero instead of the resolved value. It's still causing issues in jQuery and jQuery UI: http://bugs.jqueryui.com/ticket/6869

What can I do, other than submitting a patch, to help get this fixed?
Duplicate of this bug: 1074473
Mike, short of submitting a patch, not much you can do here.  :(

David, is this something we want to fix in GetComputedStyle, or on the layout end where GetUsedMargin() is stored in nsHTMLReflowState?  That has an XXX comment about not handling auto...
Flags: needinfo?(dbaron)
I suspect it would be a good idea to fix it on the layout side (nsHTMLReflowState, maybe other places).
Flags: needinfo?(dbaron)
Mentor: bzbarsky
It seems to have been fixed by bug 1298008?
Indeed, good catch!
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1298008
You need to log in before you can comment on or make changes to this bug.