getPropertyValue for border returns null when a computed shorthand value should be returned. This is visible in the Inspector.
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.
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.)
(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". :)
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.
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.
Comment on attachment 21451 [details] Improved patch + for (int i = 0; i < nrprops;++i) { Missing space after second colon. Looks fine. r=me
Landed in r34292.
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 on attachment 21451 [details] Improved patch Clearing darin's r+ since this has landed and I reopened the bug.
(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!
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.
(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.
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.
Comment on attachment 21467 [details] Updated layout test (save to LayoutTests/fast/css/) Marking for review to get feedback.
Comment on attachment 21467 [details] Updated layout test (save to LayoutTests/fast/css/) rs=me
Copying other interested parties.
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.
*** Bug 23159 has been marked as a duplicate of this bug. ***