Make LayoutText::absoluteQuadsForRange to consider leading/trailing whitespaces

Make LayoutText::absoluteQuadsForRange to consider leading/trailing whitespaces

This patch makes |LayoutText::absoluteQuadsForRange| to consider leading and
trailing whitespaces by adjusting specified offsets by rendered portion of
characters.

Before this patch, |LayoutText::absoluteQuadsForRange| returns empty rectangle
for character offsets not in |InlineTextBox| objects.

BUG=389054
TEST=LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces.html

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

[yosin_UTC9, 2014-06-30 04:57:36]

Could you review this patch?
Thanks in advance.

[yosin_UTC9, 2014-07-09 01:09:27]

ping...

[Julien - ping for review, 2014-07-12 00:19:42]

Levi or Emil should look at this change

https://codereview.chromium.org/361683002/diff/1/LayoutTests/fast/dom/Range/g...
File
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces-expected.txt
(right):

https://codereview.chromium.org/361683002/diff/1/LayoutTests/fast/dom/Range/g...
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces-expected.txt:18:
PASS "12"; range.getClientRects().length is 1
It seems like it would be worth actually checking what the result would be
instead of just that we have a rect.

[yosin_UTC9, 2014-07-14 02:30:39]

PTAL

https://codereview.chromium.org/361683002/diff/1/LayoutTests/fast/dom/Range/g...
File
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces-expected.txt
(right):

https://codereview.chromium.org/361683002/diff/1/LayoutTests/fast/dom/Range/g...
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces-expected.txt:18:
PASS "12"; range.getClientRects().length is 1
On 2014/07/12 00:19:42, Julien Chaffraix - PST wrote:
> It seems like it would be worth actually checking what the result would be
> instead of just that we have a rect.

Done.

[yosin_UTC9, 2015-08-18 07:41:14]

PTAL

Oops, I forgot to proceed this patch one year... :-<

[eae, 2015-08-18 16:14:46]

Thanks for doing this, I like the change but have some concerns about the test.

https://codereview.chromium.org/361683002/diff/40001/LayoutTests/fast/dom/Ran...
File LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces.html
(right):

https://codereview.chromium.org/361683002/diff/40001/LayoutTests/fast/dom/Ran...
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces.html:8:
var results = [0, 1, 2, 3, 4, 5, 6, 7, 8].map(function(offset) {
Please avoid doing data driven tests, while compact it makes it harder to
understand what the test is supposed to test and makes debugging much trickier
than it needs to be.

https://codereview.chromium.org/361683002/diff/40001/Source/core/layout/Layou...
File Source/core/layout/LayoutText.cpp (right):

https://codereview.chromium.org/361683002/diff/40001/Source/core/layout/Layou...
Source/core/layout/LayoutText.cpp:446: // Skip leading and trailing unrendered
whitespaces
The comment doesn't explain how this accounts for whitespace, presumably the
caret max/min offset does that?

[yosin_UTC9, 2015-08-19 06:08:55]

PTAL

https://codereview.chromium.org/361683002/diff/40001/LayoutTests/fast/dom/Ran...
File LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces.html
(right):

https://codereview.chromium.org/361683002/diff/40001/LayoutTests/fast/dom/Ran...
LayoutTests/fast/dom/Range/getClientRects-leading-trailing-whitespaces.html:8:
var results = [0, 1, 2, 3, 4, 5, 6, 7, 8].map(function(offset) {
On 2015/08/18 16:14:45, eae wrote:
> Please avoid doing data driven tests, while compact it makes it harder to
> understand what the test is supposed to test and makes debugging much trickier
> than it needs to be.

Done.

https://codereview.chromium.org/361683002/diff/40001/Source/core/layout/Layou...
File Source/core/layout/LayoutText.cpp (right):

https://codereview.chromium.org/361683002/diff/40001/Source/core/layout/Layou...
Source/core/layout/LayoutText.cpp:446: // Skip leading and trailing unrendered
whitespaces
On 2015/08/18 16:14:45, eae wrote:
> The comment doesn't explain how this accounts for whitespace, presumably the
> caret max/min offset does that?

Done.

[yosin_UTC9, 2015-08-19 06:09:01]

The CQ bit was checked by yosin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 06:09:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/361683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/361683002/60001

[commit-bot: I haz the power, 2015-08-19 07:36:45]

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

[commit-bot: I haz the power, 2015-08-19 07:36:45]

Dry run: This issue passed the CQ dry run.

[eae, 2015-08-19 15:38:59]

LGTM

Very nice, thank you!

[eae, 2015-08-19 15:39:05]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2015-08-19 15:39:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/361683002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/361683002/60001

[commit-bot: I haz the power, 2015-08-19 15:43:40]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200828