319381 - [FIX]getComputedStyle/getPropertyValue returns wrong overflow values

Status: RESOLVED FIXED

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

Description

[Andy Clark]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

When an element's overflow style property is set to "-moz-scrollbars-vertical" or "-moz-scrollbars-horizontal", the computed overflow value returns "0px".

For example:

  var div = document.createElement("DIV");
  div.style.overflow = "-moz-scrollbars-vertical";

  var css = window.getComputedStyle(div,"");
  alert(css.getPropertyValue("overflow"));

To work around this problem, application code must identify the browser as Firefox 1.5+ and if the overflow value is returned as "0px", then query the "overflow-x" value instead. If the overflow-x value is "hidden", then the overflow value should be "-moz-scrollbars-vertical", else it should be "-moz-scrollbars-horizontal".

Reproducible: Always

Steps to Reproduce:
1. Document contains element with overflow style property set to "-moz-scrollbars-vertical" or "-moz-scrollbars-horizontal" either using style attribute or CSS selector.
2. Call window.getComputedStyle on element.
3. Call getPropertyValue("overflow") on cssDecl object.

Actual Results:  
Calling getPropertyValue("overflow") returns "0px".

Expected Results:  
Expect getPropertyValue("overflow") to return "-moz-scrollbars-vertical" and "-moz-scrollbars-horizontal", respectively.

Comment 1

[Andy Clark]

Attachment: Sample web page that demonstrates the problem

The sample generates a matrix of overflow values and the computed values for "overflow", "overflow-x", and "overflow-y" in each case.

Comment 2

[PikeUK]

->Core:Style

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Andy, thanks for the excellent testcase!

Comment 5

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

To be honest, I think I prefer it the way it was.

Calling getComputedStyle and asking for the value of a shorthand property isn't always supposed to work.  (Although I'm skeptical that 0px is the right thing to do in that case; there are potentially a lot of cases where it could happen as properties become shorthands in later levels of CSS.)

I'd rather treat these values as the honest soon-to-be-css3-we-hope shorthand than these funky -moz- values that never should have existed.

I think we need to figure out a compatibility story for the CSS2 properties that are becoming shorthands in higher levels of CSS.  And I think whatever the answer to that general question is should be the fix for this bug -- not reuse of -moz-* values that will fix only this one case and not the general problem.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Our current general solution for shorthands is to simply not support them (since we implement getPropertyValue in terms of getPropertyCSSValue, and the latter is supposed to return nothing for shorthands)....   That really seems suboptimal to me too, for what it's worth.

All that aside, what possible value _could_ we report here?  The corresponding CSS3 value (that we don't actually support yet)?

Comment 7

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

In many cases there is no possible shorthand value to return.  For example, given:

  border-top: thick dashed blue;
  border-right: medium dotted green;
  border-bottom: none;
  border-left: thin solid red;

there's no possible computed value for the shorthand "border" property.  That's also the case for the "font" shorthand if "font-size-adjust" or "font-stretch" is specified.  CSS3 drafts also introduce this problem for existing non-shorthand properties "white-space" (for good reason) and "vertical-align" (where I hope it will go away), "display" (if we ever sort out the problems with the split) and probably a bunch of others I'm forgetting.

Comment 8

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

(In reply to comment #6)
> The corresponding
> CSS3 value (that we don't actually support yet)?

comment 7 was an attempt to answer this question -- or rather, point out that such value does not now and need not ever exist.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Just output the empty string

Comment 10

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

Comment on attachment 232755 [details] [diff] [review]
Just output the empty string

So what I don't like about this is that it looks like the only place where we return a *null* value rather than an uninitialized one.  Do you think that's really what we want to do here?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Looking at GetPropertyCSSValue(), we return null for shorthands in general, no?

Comment 12

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

Comment on attachment 232755 [details] [diff] [review]
Just output the empty string

ah, ok

r+sr=dbaron

Comment 13

[Boris Zbarsky [:bzbarsky]]

Fix checked in; still need to write a regression test.

Comment 14

[Anne (:annevk)]

FWIW, if there's more issues like this with the "CSSOM" feel free to raise them on [e-mail]. http://dev.w3.org/cvsweb/csswg/cssom/ has my ongoing draft to sort out the "CSSOM" and related functionality.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in mochitest testcase.