When an element is not displayed (display:none) and no padding has been set the computed padding is returned as 'auto', which is not a valid value for padding.
Created attachment 15982 [details] Testcase Should say "0px"
This is probably just a couple line fix in CSSComputedStyleDeclaration.cpp
This will be simple to fix. More interesting is what the correct behavior (as defined by what FF does) in the cases where it is display: none and padding is specified (either as px or as a %). Also, what happens when a table cell is display: none yet the enclosing table has cellpadding specified? We might have to break RenderObject::padding() out into a free function which an be passed a renderer and style (if available). Also interesting to look at is other CSSComputedStyleDelcaration examples where if (renderer) is checked, do those cases return invalid css values when !renderer? Again, the fix will be simple, we just need some better test cases.
If firefox just returns 0px for all test cases where the element is display: none than our job is really easy here. :) we just change the !renderer case to return new CSSPrimitiveValue(0, CSSPrimitiveValue::CSS_PX); I don't suspect it will be quite that easy.
It is that easy.
Created attachment 18506 [details] testcase (not standalone, place in fast/css to run) My fears weren't entirely unfounded! Here's FF's results from this test: PASS window.getComputedStyle(test0, null).paddingTop is "0px" FAIL window.getComputedStyle(test1, null).paddingTop should be 0px. Was 10px. FAIL window.getComputedStyle(test2, null).paddingTop should be 0px. Was 10px. FAIL window.getComputedStyle(test3, null).paddingTop should be 0px. Was 1px.
I still think 0 is a perfectly acceptable result for now. Auto isn't even a legal value for padding, so I think we make good progress if we just return 0. Actually returning the real padding would involve resolving style, and we can't really do that until we have some kind of undisplayed style cache.
An enhanced test case, and the results from various browsers: http://paste.lisp.org/display/54381
(In reply to comment #7) > Actually returning the real padding would involve resolving style, and we can't > really do that until we have some kind of undisplayed style cache. We have that.
Created attachment 18507 [details] better test case (place in fast/css to run) Results from this test case: Opera 9: PASS window.getComputedStyle(test0, null).paddingTop is "0px" PASS window.getComputedStyle(test1, null).paddingTop is "10px" PASS window.getComputedStyle(test2, null).paddingTop is "10px" PASS window.getComputedStyle(test3, null).paddingTop is "10px" PASS window.getComputedStyle(test4, null).paddingTop is "10px" PASS window.getComputedStyle(test5, null).paddingTop is "10px" PASS window.getComputedStyle(test6, null).paddingTop is "10px" PASS window.getComputedStyle(test7, null).paddingTop is "10px" PASS successfullyParsed is true TEST COMPLETE FF3b2: PASS window.getComputedStyle(test0, null).paddingTop is "0px" PASS window.getComputedStyle(test1, null).paddingTop is "10px" PASS window.getComputedStyle(test2, null).paddingTop is "10px" PASS window.getComputedStyle(test3, null).paddingTop is "10px" PASS window.getComputedStyle(test4, null).paddingTop is "10px" PASS window.getComputedStyle(test5, null).paddingTop is "10px" FAIL window.getComputedStyle(test6, null).paddingTop should be 10px. Was 0px. FAIL window.getComputedStyle(test7, null).paddingTop should be 10px. Was 0px. PASS successfullyParsed is true TEST COMPLETE Safari3: FAIL window.getComputedStyle(test0, null).paddingTop should be 0px. Was auto. PASS window.getComputedStyle(test1, null).paddingTop is "10px" PASS window.getComputedStyle(test2, null).paddingTop is "10px" FAIL window.getComputedStyle(test3, null).paddingTop should be 10px. Was auto. PASS window.getComputedStyle(test4, null).paddingTop is "10px" PASS window.getComputedStyle(test5, null).paddingTop is "10px" FAIL window.getComputedStyle(test6, null).paddingTop should be 10px. Was 10%. FAIL window.getComputedStyle(test7, null).paddingTop should be 10px. Was 10%. PASS successfullyParsed is true TEST COMPLETE
Created attachment 18515 [details] Here's the fix. Not ready for review yet since I need to make tests and stuff.
Created attachment 18530 [details] Patch including layout tests and changelog
Fixed in r29647. Thanks for the test cases.