CANCEL: Make Document::caretRangeFromPoint() to work with :before style

Make Document::caretRangeFromPoint() to work with :before style

This patch is a part of fixing issue 269556 by handling pseudo element in EventHandler.

This patch makes Document::caretRangeFromPoint() to work with :before style by changing HitTestResult class to handle pseudo element in position() and renderer() member functions.

There is similar patch for fixing caretRangeFromPoint() for :first-letter style: https://codereview.chromium.org/25668008

BUG=269556
TEST=LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html

[yosin_UTC9, 2013-11-29 23:12:38]

Could you review this patch?
Thanks in advance.

[ojan, 2013-12-02 22:39:54]

Is returning null really the behavior we want? I'd expect clicking on a
psuedo-element to do the same behavior as clicking on an unselectable element.
In the latter case, don't we just stick the cursor in the first selectable
position after the unselectable element?

https://codereview.chromium.org/96483003/diff/30001/LayoutTests/fast/dom/Docu...
File
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html
(right):

https://codereview.chromium.org/96483003/diff/30001/LayoutTests/fast/dom/Docu...
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html:9:
<div id="console"></div>
Nit: you don't need this element. js-test will inject it for you.

https://codereview.chromium.org/96483003/diff/30001/Source/core/rendering/Hit...
File Source/core/rendering/HitTestResult.cpp (right):

https://codereview.chromium.org/96483003/diff/30001/Source/core/rendering/Hit...
Source/core/rendering/HitTestResult.cpp:125: return renderer ?
renderer->positionForPoint(localPoint()) : Position();
Nit: I'd write this as follows:

if (RenderObject* renderer = this->renderer())
    return renderer->positionForPoint(localPoint());
return Position();

[yosin_UTC9, 2013-12-03 05:13:00]

PTAL

https://codereview.chromium.org/96483003/diff/30001/LayoutTests/fast/dom/Docu...
File
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html
(right):

https://codereview.chromium.org/96483003/diff/30001/LayoutTests/fast/dom/Docu...
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html:9:
<div id="console"></div>
On 2013/12/02 22:39:54, ojan wrote:
> Nit: you don't need this element. js-test will inject it for you.

Done.

https://codereview.chromium.org/96483003/diff/30001/Source/core/rendering/Hit...
File Source/core/rendering/HitTestResult.cpp (right):

https://codereview.chromium.org/96483003/diff/30001/Source/core/rendering/Hit...
Source/core/rendering/HitTestResult.cpp:125: return renderer ?
renderer->positionForPoint(localPoint()) : Position();
On 2013/12/02 22:39:54, ojan wrote:
> Nit: I'd write this as follows:
> 
> if (RenderObject* renderer = this->renderer())
>     return renderer->positionForPoint(localPoint());
> return Position();

Done.

[ojan, 2013-12-03 23:48:22]

You didn't respond to my main questions...

Is returning null really the behavior we want? I'd expect clicking on a
psuedo-element to do the same behavior as clicking on an unselectable element.
In the latter case, don't we just stick the cursor in the first selectable
position after the unselectable element?

https://codereview.chromium.org/96483003/diff/50001/LayoutTests/fast/dom/Docu...
File
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html
(right):

https://codereview.chromium.org/96483003/diff/50001/LayoutTests/fast/dom/Docu...
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html:9:
<div id="xconsole"></div>
Why leave this element in the page at all?

[esprehn, 2013-12-03 23:50:42]

Clicking a pseudo should definitely put the caret right after it. I think
Firefox does something a little different, but our behavior was better.

[yosin_UTC9, 2013-12-04 01:36:51]

On 2013/12/03 23:48:22, ojan wrote:
> You didn't respond to my main questions...
> 
> Is returning null really the behavior we want? I'd expect clicking on a
> psuedo-element to do the same behavior as clicking on an unselectable element.
> In the latter case, don't we just stick the cursor in the first selectable
> position after the unselectable element?

Oops, sorry... My brain was damaged yesterday.

Clicking on pseudo element puts caret at and element having pseudo-element.
Returning null for pseudo element by Document::caretRangeFromPoint(), is as same
as FireFox's. Although, this means clicking and caretRangeFromPoint() behave
differently.

I agree it is better that clinking and careRangeFromPoint() behave same
including :after, which puts caret at end of element.

> 
>
https://codereview.chromium.org/96483003/diff/50001/LayoutTests/fast/dom/Docu...
> File
>
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html
> (right):
> 
>
https://codereview.chromium.org/96483003/diff/50001/LayoutTests/fast/dom/Docu...
>
LayoutTests/fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-with-before-style.html:9:
> <div id="xconsole"></div>
> Why leave this element in the page at all?

[yosin_UTC9, 2013-12-04 01:37:23]

On 2013/12/03 23:50:42, esprehn wrote:
> Clicking a pseudo should definitely put the caret right after it. I think
> Firefox does something a little different, but our behavior was better.

I agree. I'll update to do so.

[yosin_UTC9, 2013-12-04 01:54:13]

Note: putting caret to non-pseudo element is done by VisiblePosition in
EventHandler.