Bug 15823 - getPropertyValue for border returns null, should compute the shorthand value
: getPropertyValue for border returns null, should compute the shorthand value
Status: RESOLVED FIXED
: WebKit
CSS
: 523.x (Safari 3)
: All All
: P3 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2007-11-03 18:27 PST by
Modified: 2009-01-07 02:15 PST (History)


Attachments
First attempt (3.97 KB, patch)
2008-05-03 09:13 PST, Rob Buis
ddkilzer: review-
Review Patch | Details | Formatted Diff | Diff
Improved patch (7.00 KB, patch)
2008-05-31 08:47 PST, Rob Buis
no flags Review Patch | Details | Formatted Diff | Diff
Updated layout test (save to LayoutTests/fast/css/) (3.31 KB, text/plain)
2008-06-02 13:56 PST, David Kilzer (:ddkilzer)
darin: review+
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2007-11-03 18:27:14 PST
getPropertyValue for border returns null when a computed shorthand value should be returned. This is visible in the Inspector.
------- Comment #1 From 2008-05-03 09:13:25 PST -------
Created an attachment (id=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.
------- Comment #2 From 2008-05-28 06:42:45 PST -------
(From update of attachment 20947 [details])
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.)
------- Comment #3 From 2008-05-28 06:43:47 PST -------
(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".  :)
------- Comment #4 From 2008-05-30 13:32:08 PST -------
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.
------- Comment #5 From 2008-05-31 08:47:35 PST -------
Created an attachment (id=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.
------- Comment #6 From 2008-06-01 12:35:49 PST -------
(From update of attachment 21451 [details])
+            for (int i = 0; i < nrprops;++i) {

Missing space after second colon.

Looks fine. r=me
------- Comment #7 From 2008-06-01 23:45:57 PST -------
Landed in r34292.
------- Comment #8 From 2008-06-02 11:15:49 PST -------
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"
------- Comment #9 From 2008-06-02 11:22:20 PST -------
(From update of attachment 21451 [details])
Clearing darin's r+ since this has landed and I reopened the bug.
------- Comment #10 From 2008-06-02 11:57:58 PST -------
(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!
------- Comment #11 From 2008-06-02 12:18:37 PST -------
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.
------- Comment #12 From 2008-06-02 13:07:12 PST -------
(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.
------- Comment #13 From 2008-06-02 13:56:50 PST -------
Created an attachment (id=21467) [details]
Updated layout test (does not work from here)

(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.
------- Comment #14 From 2008-06-21 12:10:36 PST -------
(From update of attachment 21467 [details])
Marking for review to get feedback.
------- Comment #15 From 2008-06-23 10:22:55 PST -------
(From update of attachment 21467 [details])
rs=me
------- Comment #16 From 2008-06-23 13:35:06 PST -------
Copying other interested parties.
------- Comment #17 From 2008-06-23 13:54:01 PST -------
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.
------- Comment #18 From 2009-01-07 02:15:45 PST -------
*** Bug 23159 has been marked as a duplicate of this bug. ***