Bug 32230

Hi, We've identified the same problem within our unit and manual tests for appearance methods. Firefox, Opera and IE all report the values accurately, but Webkit browsers will return a zoomed+orig value. For example, create 2 elements with text inside. Set the first to be 16px in size, and then use JS to apply get element 1's font-size value and apply it to element 2. When normal zoom, both elements will appear as 16px, but zoom the page by 1, and element 2 will be larger than element 1 in a webkit browser. Of course, the issue becomes more pronounced the further the user zooms. Thanks, Frances http://bbc.co.uk/glow

Bug 25750 solved the same problem regarding getBoundingClientRects so I'm CC:ing some people from that list. Gecko returns the same computed value no matter the zoom level so there should be no doubt what the right behavior is.

Created attachment 58483 [details] better test case

I looked at the code for this and I think a solution would be to replace all calls like this in CSSComputedStyleDeclaration.cpp CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_PX); with something like createZoomAdjustedPixelValue(value, renderer) which would call WebCore::adjustForLocalZoom on the value and then return a new CSSPrimitiveValue.

Bug 42089 is somewhat related.

Created attachment 67278 [details] Patch

Comment on attachment 67278 [details] Patch Since the patch doesn’t apply, the EWS was unable to analyze it.

Comment on attachment 67278 [details] Patch I'll upload a new patch without the merge conflicts.

Created attachment 67576 [details] Fixed merge conflicts

Comment on attachment 67576 [details] Fixed merge conflicts View in context: https://bugs.webkit.org/attachment.cgi?id=67576&action=prettypatch Great! I’m a little worried about the rounding caused by zooming and then scaling back, but I like how you are fitting this into the existing code. > WebCore/css/CSSComputedStyleDeclaration.cpp:336 > + // Needed because the pixel value truncates (rather than rounds) when scaling up. > + if (zoomFactor > 1) > + value++; I’m not sure this fudge factor is going to do the exact right thing in all cases. Is adding exactly one the right thing to do? Can we make a test case that is right on the edge of the rounding? > WebCore/css/CSSComputedStyleDeclaration.cpp:391 > + if (l.type() == WebCore::Fixed) > + return zoomAdjustedPixelValue(l.value(), style); Looks like the indentation is wrong here. The return should be indented four spaces. I am not going to mark this review+ yet because that will cause the EWS bots to not process the patch!

Created attachment 67621 [details] Style fixes and reuse zoom adjusting code from RenderObject.h

(In reply to comment #10) > (From update of attachment 67576 [details]) > I’m not sure this fudge factor is going to do the exact right thing in all cases. Is adding exactly one the right thing to do? Can we make a test case that is right on the edge of the rounding? This is the same code as used elsewhere for adjusting the zoom value. I refactored the code to use the same code.

Comment on attachment 67621 [details] Style fixes and reuse zoom adjusting code from RenderObject.h View in context: https://bugs.webkit.org/attachment.cgi?id=67621&action=prettypatch Looks good. > WebCore/css/CSSComputedStyleDeclaration.cpp:341 > + double zoomFactor = style->effectiveZoom(); > + if (zoomFactor != 1) > + value = value / zoomFactor; > + > + return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER); Why the if statement and special handling of 1? Is it a performance optimization? This would read better if it just said value / style->effectiveZoom() without a local variable.

Fixed in http://trac.webkit.org/changeset/67568

I think this broke http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/textCapitalizeEdgeCases.html on Leopard.

(In reply to comment #15) > I think this broke http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/textCapitalizeEdgeCases.html on Leopard. I don't think it broke that one but I'm currently fixing these 2 tests animations/fill-mode-transform.html editing/pasteboard/page-zoom.html

http://trac.webkit.org/changeset/67568 might have broken Qt Linux Release

Created attachment 70033 [details] Occurrence of the bug after the first fix with display:none This issue still occurs when an element (or its ancestor) has display:none.

Reopening to fix up more of the cases.

<rdar://problem/9025697>

Created attachment 126390 [details] Patch

Committed r107301: <http://trac.webkit.org/changeset/107301>

I think there still might be a problem here... but this time with the computed value for 'left'. An example of the issue can be seen here: http://jsfiddle.net/sFYjc/2/ (The jQuery community has stumbled upon the issue a few times over the years... http://bugs.jquery.com/ticket/4993#comment:9 ) I am running Chrome 23.0.1271.97 m.

Ideally we'd open a new bug with a test case and it can be resolved there. :)

No problem - done: https://bugs.webkit.org/show_bug.cgi?id=105979