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

Status: RESOLVED DUPLICATE of bug 1298008

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

Description

[Eli Friedman]

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.

Comment 1

[Eli Friedman]

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.

Comment 2

[John J. Barton]

See also Bug 258255 and 356665.  This bug makes workaround for those bugs more difficult. Maybe the have the same root cause.

Comment 3

[Justin Dolske [:Dolske]]

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]

Comment 4

[T.Rosenau]

(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 )

Comment 5

[T.Rosenau]

(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().

Comment 6

[Rob Campbell [:rc] (:robcee)]

Attachment: getComputedStyle testcase

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?

Comment 7

[Joel Dart]

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

Comment 8

[Mike Sherov]

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.

Comment 9

[Mike Sherov]

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.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We need to test this for all values of 'position'.

Comment 12

[Mike Petrovich]

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.

Comment 13

[Paul Rouget [:paul]]

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.

Comment 14

[Mike Sherov]

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?

Comment 16

[Boris Zbarsky [:bzbarsky]]

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...

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I suspect it would be a good idea to fix it on the layout side (nsHTMLReflowState, maybe other places).

Comment 18

[nayinain]

It seems to have been fixed by bug 1298008?

Comment 19

[Boris Zbarsky [:bzbarsky]]

Indeed, good catch!