Problem #1 Depending on the 2nd argument,getComputedStyle seems to measure from the element's border edge or padding edge. Its my understanding that computed values are derived from the specified values. The style properties measure from the margin edge and I think the computed values should reflect this. Problem #2 The right and bottom are calculated from the left/top of the parent instead of the right/bottom,this is incorrect behavior according to CSS2 specs

Harish, could you look into this, especially problem #2?

To clarify problem # 2 The correct way to measure for the right property (according to the specs),would be to go from the right edge of the child to the right edge of the parent. getComputedStyle() measures from the parent's left edge to the child's right edge. The same kinda thing happens with the bottom property.

--> 0.9.5

Moving back to 0.9.4.

ain't going to make it to 0.9.4 train. Moving to 0.9.5.

I do have a fix for the right property but haven't gotten the time to test it :-( Will look into it at the earliest possible.

Handing over getComputedStyle bugs to bzbarsky. Thanks boris.

Problem #1 is bug 73525. Working on #2.

To clarify problem #1 To get the computed top value of an abs. pos. element,you would measure from the element's top margin edge to the top padding edge of the offsetParent (assuming it is block level). getComputedStyle() does not measure from the element's margin edge, it measures from the border or padding edge (depending on the 2nd argument). It should measure from the margin edge of the child.

The attached patch fixes positioning properties for all cases I could think of _except_: 1) left/right/top/bottom for statically positioned elements (returns NS_ERROR_NOT_IMPLEMENTED at the moment) 2) width/height for non-replaced inline elements (returns NS_ERROR_NOT_IMPLEMENTED at the moment) Tested on the attached testcases as well as using DOM inspector on http://web.mit.edu/bzbarsky/www/books.html (for a static positioning with scrollbar testcase). reviews?

Comment on attachment 55568 [details] [diff] [review] Patch to fix most of this stuff Forgot a null-check

glazou, would you care to take a look?

Boris, In your test case for static positioning,you show that some properties should return values in pixels and some in different units. Is getPropertyValue() suppose to sniff out the the unit used in the styling and return the appropriate value? In the past these values were only returned in pixels.

Well... The static position case is odd because those properties get ignored (and hence not computed) for statically positioned elements. Depending on who you ask, we're supposed to return any of: 1) 0 2) The value set in the sheet converted into pixels 3) The value set in the sheet converted into pixels unless it's a percentage value, in which case we should return the percentage value (eg "15%") 4) The value set in the sheet with no modifications My testcase was written to cover case 4, which is the hardest of the set to implement. If you note, I actually make those properties throw an exception on getting, pending decision on what should be done with them. Any useful info on that would be much appreciated.

just one thing : nsComputedDOMStyle::GetContainingBlock's name should be changed since it looks for the first containing frame which does not have %age-based size. Continuing the review.

Comment on attachment 55999 [details] [diff] [review] Doh. Missed a null-check in GetZIndex() >Index: content/shared/src/nsStyleStruct.cpp >... >+ if (borderData) { >+ nsMargin border; >+ borderData->CalcBorderFor(frame, border); >+ baseWidth -= (border.left + border.right); >+ } only indentation. >+ nsIFrame* GetContainingBlock(nsIFrame *aFrame); I wonder if nsComputedDOMStyle is really the place for this and if it could not be shared with other pieces of code. Looks fine otherwise ! r=glazman

> since it looks for the first containing frame which does not have %age-based > size. Actually, it does not. It looks for the first containing frame that is positioned or a block. The magic comes from the frame hierarchy -- positioned elements get reparented to their containing block (hence placeholder frames). And yes, this function _does_ duplicate existing reflow functionality, as does half this code. That's the subject of bug 106154. Indentation fixed. Thanks for the review, Daniel!

Didn't make it. pushing out.

Comment on attachment 55999 [details] [diff] [review] Doh. Missed a null-check in GetZIndex() sr=attinasi

Checked in. Thanks for the reviews!