Bug 15823 – getPropertyValue for border returns null, should compute the shorthand value

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Timothy Hatcher]

getPropertyValue for border returns null when a computed shorthand value should be returned. This is visible in the Inspector.

[Rob Buis]

Created attachment 20947 [details]
First attempt

This patch fixes getPropertyValue("border"). I think it is okay to make another
code path for border as it is the only shorthand property AFAIK that consists of only other shorthand properties.
Cheers,

Rob.

[David Kilzer (:ddkilzer)]

Comment on attachment 20947 [details]
First attempt

What happens when you try to get the "border" property of a div like this?

  <div id="border" style="border-left: 5px solid red; border-top: 5px solid green; border-right: 5px solid blue; border-bottom: 5px solid purple; " ></div>

I think it returns "5px 5px 5px 5px solid solid solid solid green blue purple red", which is not a legal value for the "border" property.

Does the CSS spec say what should happen in the case where a single "border" property can't accurately represent an element's border?

I tried the above test case in Firefox 2.0.0.x, and it returns the value for "border-top" when asking for the "border" property!  Not sure what MSIE 6 or 7 do.

r- since this change doesn't return a value valid in the negative case.  (We should at least have a "negative" test case as well.)

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> r- since this change doesn't return a value valid in the negative case.  (We
> should at least have a "negative" test case as well.)

Or a "valid value".  :)

[Rob Buis]

Hi David,

(In reply to comment #2)
> (From update of attachment 20947 [details] [edit])
> What happens when you try to get the "border" property of a div like this?
> 
>   <div id="border" style="border-left: 5px solid red; border-top: 5px solid
> green; border-right: 5px solid blue; border-bottom: 5px solid purple; " ></div>
> 
> I think it returns "5px 5px 5px 5px solid solid solid solid green blue purple
> red", which is not a legal value for the "border" property.

Correct, good find.

> Does the CSS spec say what should happen in the case where a single "border"
> property can't accurately represent an element's border?

Will look into that.

> I tried the above test case in Firefox 2.0.0.x, and it returns the value for
> "border-top" when asking for the "border" property!  Not sure what MSIE 6 or 7
> do.

Opera does "5px solid".
 
> r- since this change doesn't return a value valid in the negative case.  (We
> should at least have a "negative" test case as well.)

Ok, will work on that. Thanks for the review!
Cheers,

Rob.

[Rob Buis]

Created attachment 21451 [details]
Improved patch

The testcase mentioned by David is now included, as well as one where border is set and then overridden. The behaviour with patch matches that of Opera.
Cheers,

Rob.

[Darin Adler]

Comment on attachment 21451 [details]
Improved patch

+            for (int i = 0; i < nrprops;++i) {

Missing space after second colon.

Looks fine. r=me

[Rob Buis]

Landed in r34292.

[David Kilzer (:ddkilzer)]

Rob, what happens in this case?

<div id="border4" style="border-left: 4px solid green; border-top: 5px solid green; border-right: 5px solid green; border-botton: 5px solid green;" ></div>

I think that's going to produce this value, which would not be correct:

"   solid green"

[David Kilzer (:ddkilzer)]

Comment on attachment 21451 [details]
Improved patch

Clearing darin's r+ since this has landed and I reopened the bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Rob, what happens in this case?
> 
> <div id="border4" style="border-left: 4px solid green; border-top: 5px solid
> green; border-right: 5px solid green; border-botton: 5px solid green;" ></div>
> 
> I think that's going to produce this value, which would not be correct:
> 
> "   solid green"

Actually, that's "solid green", and apparently that's supported!

[Rob Buis]

Hi David,

(In reply to comment #10)
> (In reply to comment #8)
> > Rob, what happens in this case?
> > 
> > <div id="border4" style="border-left: 4px solid green; border-top: 5px solid
> > green; border-right: 5px solid green; border-botton: 5px solid green;" ></div>

It is border-botton above. Even the spelling indicates that ;)

> > I think that's going to produce this value, which would not be correct:
> > 
> > "   solid green"
> 
> Actually, that's "solid green", and apparently that's supported!

I must admit I did not test that before landing, but now I did indeed it
returns "solid green", like Opera. I guess the report can be reclosed now?
Cheers,

Rob.

[David Kilzer (:ddkilzer)]

(In reply to comment #11)
> I must admit I did not test that before landing, but now I did indeed it
> returns "solid green", like Opera. I guess the report can be reclosed now?

Thanks, Rob

I'm going to update the layout test to cover all permutations of mismatched border values.

[David Kilzer (:ddkilzer)]

Created attachment 21467 [details]
Updated layout test (save to LayoutTests/fast/css/)

(In reply to comment #12)
> I'm going to update the layout test to cover all permutations of mismatched
> border values.

New layout test produces this results with ToT WebKit:

PASS div1.style.getPropertyValue("border") is '5px solid green'
PASS div2.style.getPropertyValue("border") is '5px solid'
FAIL div3.style.getPropertyValue("border") should be 5px. Was 5px green.
PASS div4.style.getPropertyValue("border") is 'solid green'
FAIL div5.style.getPropertyValue("border") should be null (of type object). Was green (of type string).
PASS div6.style.getPropertyValue("border") is '5px'
PASS div7.style.getPropertyValue("border") is 'solid'
PASS div8.style.getPropertyValue("border") is null
PASS div9.style.getPropertyValue("border") is null
PASS successfullyParsed is true

Note that the following list of values are acceptable for the "border" style:

border-width
border-width border-style
border-width border-style border-color
border-style
border-style border-color

While the following are not valid (in ToT WebKit, Opera 9.27 and Firefox 2.0.0.x):

border-color
border-width border-color

Should we return "null" in the above two cases?  Opera 9.27 returns the two invalid values, even thought they can't be used in a new declaration.

[David Kilzer (:ddkilzer)]

Comment on attachment 21467 [details]
Updated layout test (save to LayoutTests/fast/css/)

Marking for review to get feedback.

[Darin Adler]

Comment on attachment 21467 [details]
Updated layout test (save to LayoutTests/fast/css/)

rs=me

[David Kilzer (:ddkilzer)]

Copying other interested parties.

[David Kilzer (:ddkilzer)]

Committed updated test:

$ svn commit LayoutTests
Sending        LayoutTests/ChangeLog
Sending        LayoutTests/fast/css/getPropertyValue-border-expected.txt
Sending        LayoutTests/fast/css/getPropertyValue-border.html
Transmitting file data ...
Committed revision 34745.

[Robert Blaut]

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