New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 606662 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

range.getBoundingClientRect does not produce consistent height as in previous versions

Reported by n...@ntstdio.net, Apr 26 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.87 Safari/537.36

Steps to reproduce the problem:
* https://jsfiddle.net/shkz9hff/
* https://jsfiddle.net/f12orcmq/

What is the expected behavior?
The result I got from Chrome 49 doesn't seem to be incorrect, so I'm wondering if this is intended or a regression.

What went wrong?
The number of ClientRects[] for characters at boundary seem to be more than others, which produce a larger bounding client rect.

(1) In the first JSFiddle, say it's wrapped in this way:
012345678901
234567890123
456789

In Chrome 49.0.2623.112 I see all the output *height* (i.e. in the bottom-right pane of JSFiddle) to be 17. In 50.0.2661.87 I see the height as 35 in idx 12 ('2') and 24 ('4').

(2) In the second JSFiddle, say it's wrapped in this way:
0123456789
0123456799
0123456789

Similar thing occurs -- at idx 10 (' ') and idx 21 (' ') the height is 35 in Chrome 50.

Did this work before? Yes At least Chrome 49.0.2623.112

Chrome version: 50.0.2661.87  Channel: stable
OS Version: 10.0
Flash Version: Shockwave Flash 21.0 r0

Maybe the point is why the character at (left) boundary behaves differently.
 

Comment 1 by e...@chromium.org, Apr 27 2016

Components: -Blink Blink>Layout
Labels: -OS-Windows
Owner: kojii@chromium.org
Status: Assigned (was: Unconfirmed)
This appears to be related to your fix for 456282 kojii, would you mind taking a look when you get a chance?

Comment 2 by kojii@chromium.org, Apr 28 2016

Cc: kojii@chromium.org
 Issue 607161  has been merged into this issue.

Comment 3 by kojii@chromium.org, Apr 28 2016

Labels: Needs-Bisect OS-All
Huh, this wasn't a regression from  bug 456282 , I can still reproduce after locally reverted.

Comment 4 by kojii@chromium.org, Apr 28 2016

Merged all reported tests; break-word and normal wrap from this bug, and pre-wrap from  issue 607161 :
http://output.jsbin.com/peyuro/

Chrome 49.0.2623.112: 81 pass, 13 fail
Chrome 52.0.2714.0: 78 pass, 16 fail
Firefox: 94 pass, 0 fail
IE 11: 82 pass, 12 fail
Safari: 81 pass, 13 fail

All browsers other than Firefox are broken one way or another.

In Chrome, 3 failures increased in 52. 

Comment 5 by rom...@gmail.com, Apr 28 2016

One question on the tests, I would find it incredibly useful if the multiple clientRects (i.e., only two - the original before-wrapping rect, and the post-wrapping rect) were returned for range.getClientRects() on wrapped single-character selections (as they are for multi-character selections).

The tests suggest that this case would be a failure, though – the failure in my mind is that despite having multiple clientRects, there's an "actual" clientRect and that should be represented as the boundingClientRect. Maybe the test assertions are correct, but there's another API needed here, or a parameter on getClientRects() or something.

The use-case in particular that I have is drawing a caret at a wrap boundary; depending on the location of the user's last interaction, the caret should be drawn at the coordinates before wrapping occurs, or the coordinates after wrapping occurred. Without the original location, drawing the caret correctly depends on a hack to re-synthesize the original coordinates.

Comment 6 by n...@ntstdio.net, Apr 28 2016

Hmm, looks like the output client rects in both 49 and 50 are the same. So really the changed behavior would be something as follows.

* Old
             ClientRect {top: 86, right: 283, bottom: 102, left: 283, width: 0…} 
             ClientRect {top: 107, right: 39, bottom: 123, left: 31, width: 8…} 
=> Bounding: ClientRect {top: 107, right: 39, bottom: 123, left: 31, width: 8…}

* New
             ClientRect {top: 86, right: 283, bottom: 102, left: 283, width: 0…} 
             ClientRect {top: 107, right: 39, bottom: 123, left: 31, width: 8…} 
=> Bounding: ClientRect {top: 86, right: 283, bottom: 123, left: 31, width: 252…}

I have the same use case as #5 relying on the getBoundingClientRect. Maybe this is intended?
Labels: -Needs-Bisect hasbisect M-52
Able to reproduce the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.11.4 using chrome version 50.02661.87 and canary 52.0.2719.0.As per the above comment #4 considering the build Chrome 49.0.2623.112: 81 pass, 13 fail as good providing the bisect as below

Narrow Bisect::
Good:: 50.0.2655.0  --   (official build 376333)
Bad::50.0.2656.0   ---   (official build 376674)

CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/78dc6befc74f8b7b823eb071c16ea2bc6a3c3314..a504e48570e35e6229fdcbdf7967510507a843b8

Unable to find the exact suspect from the above CL.Could any one please look into this issue.
removing the Bisect label,please feel free to add if required.
Thnaks,

Comment 8 by kojii@chromium.org, May 2 2016

#7: thanks, it's probably r376398. I'll look into further.

Comment 9 by kojii@chromium.org, May 2 2016

Cc: e...@chromium.org sim...@opera.com
Owner: sim...@opera.com
So there are two non-interoperable behavior; one the number of rects for getClientRects(), and the other getBoundingClientRect() in such cases.

Focusing on the latter, here's the current status:

Spec:
the smallest rectangle that includes the first rectangle in list and all of the remaining rectangles of which the height or width is not zero.
https://www.w3.org/TR/cssom-view-1/#extensions-to-the-range-interface

Implementations: (tested with http://output.jsbin.com/peyuro)
A. Chrome 52 (after the fix for  issue 574363 ) follows the spec.
B. Safari, Chrome 49 unions all rects if width or height is not zero without the special handling of the first rect.
C. IE unions all rects even if width or height is zero.
D. Gecko can't be determined since it does not return multiple rects in any of these cases.  Issue 574363  is talking about collapsed range, but not when multiple rects with some/all are zero width/height.

simonp@, WDYT? I guess the special handling of the first rect is to support collapsed range, but it looks like it has more side effects. Do you know when/how it was defined?

Comment 10 by sim...@opera.com, May 2 2016

Owner: kojii@chromium.org
I can find
https://github.com/w3c/csswg-drafts/commit/ccff98328b1c0019e953f82d98b52e32f2b82681
https://lists.w3.org/Archives/Public/www-style/2009Jul/0149.html

I don't have an opinion about what we should do here; I suggest discussing with other implementors. I'm happy to change the spec.
Thank you for the pointer; talked with roc and confirmed that Gecko actually does differently from the spec.

Sent a spec change proposal to:
https://lists.w3.org/Archives/Public/www-style/2016May/0045.html
or in thread view: https://readable-email.org/list/www-style/topic/cssom-view-getboundingclientrect-when-the-first-rectangle-is-empty

Project Member

Comment 12 by bugdroid1@chromium.org, May 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27e0f0178d9c3d13bcd4dc96085cd0a749081d46

commit 27e0f0178d9c3d13bcd4dc96085cd0a749081d46
Author: kojii <kojii@chromium.org>
Date: Wed May 11 21:02:51 2016

Change Range.getBoundingClientRect() when the first rect is empty

crrev.com/376398 changed Range.getBoundingClientRect() to include the
first rectangle even when it is empty (either width or height is zero.)
This change improves when all rects are empty, but produces undesirable
result when the first rectangle is empty while the rest is not.

This patch changes it to return the first rectangle when all rectangles
are empty, following the spec change in [1].

content_editable_extractor_test.unitjs was reverted to before
crrev.com/376398.

[1] https://github.com/w3c/csswg-drafts/commit/0e7a5cbdea19397086e9423b508fe6f41decdcec

BUG= 606662 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/1949373003
Cr-Commit-Position: refs/heads/master@{#393057}

[modify] https://crrev.com/27e0f0178d9c3d13bcd4dc96085cd0a749081d46/chrome/browser/resources/chromeos/chromevox/common/content_editable_extractor_test.unitjs
[add] https://crrev.com/27e0f0178d9c3d13bcd4dc96085cd0a749081d46/third_party/WebKit/LayoutTests/fast/dom/Range/get-bounding-client-rect-empty-and-non-empty.html
[modify] https://crrev.com/27e0f0178d9c3d13bcd4dc96085cd0a749081d46/third_party/WebKit/Source/core/dom/Range.cpp

Comment 13 by kojii@chromium.org, May 12 2016

Status: Fixed (was: Assigned)

Comment 14 by kojii@chromium.org, May 12 2016

All 3 tests now pass on Canary 52.0.2734.0

Sign in to add a comment