Bug 88430

Summary:

Element.getBoundingClientRect() and Element.getClientRects() return incorrect values in frames in a scaled page

Product:

WebKit

Reporter:

Xianzhu Wang <wangxianzhu>

Component:

DOM

Assignee:

Xianzhu Wang <wangxianzhu>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aelias, bdakin, cmarcelo, eric, jamesr, klobag, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	none
patch v2 (removed extra code in layout test)	none
patch v3 (using Frame::frameScaleFactor())	jamesr: review+, jamesr: commit-queue-
patch for landing	none

Xianzhu Wang

Reported 2012-06-06 10:08:58 PDT

Element.getBoudingClientRect() and Element.getClientRects() should not scale back the rects if the element is in a frame because page scale only apply to elements in the main frame.

Attachments
patch (14.80 KB, patch) 2012-06-06 11:23 PDT, Xianzhu Wang	no flags	Details Formatted Diff Diff
patch v2 (removed extra code in layout test) (14.37 KB, patch) 2012-06-06 11:35 PDT, Xianzhu Wang	no flags	Details Formatted Diff Diff
patch v3 (using Frame::frameScaleFactor()) (14.31 KB, patch) 2012-06-06 13:28 PDT, Xianzhu Wang	jamesr: review+ jamesr: commit-queue-	Details Formatted Diff Diff
patch for landing (14.33 KB, patch) 2012-06-06 16:52 PDT, Xianzhu Wang	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Xianzhu Wang

Comment 1 2012-06-06 11:23:05 PDT

Created attachment 146068 [details] patch

Xianzhu Wang

Comment 2 2012-06-06 11:35:54 PDT

Created attachment 146075 [details] patch v2 (removed extra code in layout test)

Alexandre Elias

Comment 3 2012-06-06 13:05:02 PDT

Looks like frame()->frameScaleFactor() was already introduced for this purpose in bug 68081 , maybe we can reuse that?

Xianzhu Wang

Comment 4 2012-06-06 13:28:33 PDT

Created attachment 146098 [details] patch v3 (using Frame::frameScaleFactor()) Thanks Alex for your good idea.

James Robinson

Comment 5 2012-06-06 15:39:45 PDT

Comment on attachment 146098 [details] patch v3 (using Frame::frameScaleFactor()) View in context: https://bugs.webkit.org/attachment.cgi?id=146098&action=review R=me > Source/WebCore/ChangeLog:3 > + Element.getBoudingClientRect() and Element.getClientRects() return incorrect values in frames in a scaled page typo "bouding" -> "bounding" > Source/WebCore/dom/Document.cpp:6041 > + extra newline > LayoutTests/fast/dom/Element/scale-page-bounding-client-rect-in-frame.html:1 > +<html> please use <!DOCTYPE html> for this and other new test files unless you explicitly want to test quirks mode behavior

Xianzhu Wang

Comment 6 2012-06-06 16:51:51 PDT

Comment on attachment 146098 [details] patch v3 (using Frame::frameScaleFactor()) View in context: https://bugs.webkit.org/attachment.cgi?id=146098&action=review >> Source/WebCore/ChangeLog:3 >> + Element.getBoudingClientRect() and Element.getClientRects() return incorrect values in frames in a scaled page > > typo "bouding" -> "bounding" Done (also other places) >> Source/WebCore/dom/Document.cpp:6041 >> + > > extra newline Done. >> LayoutTests/fast/dom/Element/scale-page-bounding-client-rect-in-frame.html:1 >> +<html> > > please use <!DOCTYPE html> for this and other new test files unless you explicitly want to test quirks mode behavior Done (also other places). Also changed "xxxPageScale" in the change to "xxxFrameScale" as per Grace's suggestion.

Xianzhu Wang

Comment 7 2012-06-06 16:52:07 PDT

Created attachment 146146 [details] patch for landing

WebKit Review Bot

Comment 8 2012-06-06 23:57:38 PDT

Comment on attachment 146146 [details] patch for landing Clearing flags on attachment: 146146 Committed r119690: <http://trac.webkit.org/changeset/119690>

WebKit Review Bot

Comment 9 2012-06-06 23:57:43 PDT

All reviewed patches have been landed. Closing bug.

Simon Fraser (smfr)

Comment 10 2012-12-17 20:58:01 PST

I wish we could have just fixed absoluteQuads() to not include frame scale, rather than adding code to adjustFloatQuadsForScrollAndAbsoluteZoomAndFrameScale() to remove frame scale from the quads again. We don't have a clear policy about when localToAbsolute() stuff includes frame scale, but could add one.

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