range.getBoundingClientRect() returns incorrect value for collapsed ranges

range.getBoundingClientRect() returns incorrect value for collapsed ranges

Current implementation returns empty rectangle if
either width or height is zero. But as per W3C specification [1],
getBoundingClientRect() should return union of rectangles
of which the height or width is not zero.

[1] http://www.w3.org/TR/cssom-view/#dom-range-getclientrects

BUG=435438
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192699

[Abhijeet Kandalkar Slow, 2015-03-12 12:30:17]

abhijeet.k@samsung.com changed reviewers:
+ eae@chromium.org, keishi@chromium.org, vivekg@chromium.org

[Abhijeet Kandalkar Slow, 2015-03-12 12:31:19]

Please review changes.

[eae, 2015-03-19 20:52:21]

https://codereview.chromium.org/998263002/diff/1/Source/core/dom/Range.cpp
File Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/998263002/diff/1/Source/core/dom/Range.cpp#ne...
Source/core/dom/Range.cpp:1791: result = quad.boundingBox();
This doesn't seem right, if the last quad is empty the entire rect is reset
based on the last item. Is that really the desired behavior?

[Abhijeet Kandalkar Slow, 2015-03-20 07:48:47]

Incorporated review commenets. Please have look.

https://codereview.chromium.org/998263002/diff/1/Source/core/dom/Range.cpp
File Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/998263002/diff/1/Source/core/dom/Range.cpp#ne...
Source/core/dom/Range.cpp:1791: result = quad.boundingBox();
On 2015/03/19 20:52:21, eae wrote:
> This doesn't seem right, if the last quad is empty the entire rect is reset
> based on the last item. Is that really the desired behavior?

Done.

[eae, 2015-03-20 15:37:01]

LGTM, thanks

[Abhijeet Kandalkar Slow, 2015-03-20 15:46:38]

The CQ bit was checked by abhijeet.k@samsung.com

[commit-bot: I haz the power, 2015-03-20 15:46:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/998263002/20001

[commit-bot: I haz the power, 2015-03-20 18:35:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 18:35:56]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/55954)

[Abhijeet Kandalkar Slow, 2015-03-23 09:54:11]

On 2015/03/20 18:35:56, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   win_blink_rel on tryserver.blink (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/55954)

Test needs to be rebaselined for win and mac platform.

[Abhijeet Kandalkar Gmail, 2015-03-27 12:35:12]

kandalkar.abhijeet58@gmail.com changed reviewers:
+ kandalkar.abhijeet58@gmail.com

[Abhijeet Kandalkar Gmail, 2015-03-27 12:35:12]

Hi eae,
Please have look of latest patch.

- As development platform was Linux, added test expectation for this bug is
working fine on LINUX platform and failing on WIN/MAC platform.

- Added 'NeedsRebaseline' in LayoutTests/TestExpectations

- Test result for Win-debug build :

FAIL rect.left should be 22. Was 23.
PASS rect.top is 8
PASS rect.width is 0
FAIL rect.height should be 19. Was 17.
PASS successfullyParsed is true

[eae, 2015-03-27 18:58:52]

LGTM

[Abhijeet Kandalkar Slow, 2015-03-27 19:13:13]

The CQ bit was checked by abhijeet.k@samsung.com

[commit-bot: I haz the power, 2015-03-27 19:13:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/998263002/40001

[commit-bot: I haz the power, 2015-03-27 21:01:25]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192699

[Peter Kasting, 2015-03-28 03:00:33]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1044593002/ by pkasting@chromium.org.

The reason for reverting is: Seems to have broken
CvoxShadowUnitTest.MultilineWrap in the chromevox_tests..