1397514 - Selection jumps when the mouse cursor enters an absolute/fixed position parent

Status: NEW

(Core :: DOM: Selection, defect, P3)

Description

[Javier Fernandez]

Attachment: dragSelectionExtent1.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170816210634

Steps to reproduce:

(1) Load the attached test case
(2) Start selecting inside the float box
(3) Extend selection outside the float box



Actual results:

The selection start/end points are inverted, so the highlighted text is
the one from the beginning of the float box to the selection start point. 



Expected results:

Highlighted text should match start/end visual selection point.

Comment 1

[Javier Fernandez]

This bug is re reproducible in Webkit and Blink engines as well
  - https://crbug.com/758526
  - https://webkit.org/b/101771

Comment 2

[Javier Fernandez]

I have plans to work on this issue in Webkit and Blink, but I'm worried about interoperability. What's the reason for the wont fix ?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Javier Fernandez from comment #2)
> I have plans to work on this issue in Webkit and Blink, but I'm worried
> about interoperability. What's the reason for the wont fix ?

Those are per-release tracking flags.

Note that it's only wontfix for Firefox 57 (because it's pretty unlikely that we come up with a fix during the next week given nobody is actively working on this right now).

Comment 4

[Javier Fernandez]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> (In reply to Javier Fernandez from comment #2)
> > I have plans to work on this issue in Webkit and Blink, but I'm worried
> > about interoperability. What's the reason for the wont fix ?
> 
> Those are per-release tracking flags.
> 
> Note that it's only wontfix for Firefox 57 (because it's pretty unlikely
> that we come up with a fix during the next week given nobody is actively
> working on this right now).

Oh, ok, thanks for the info.

Comment 5

[Javier Fernandez]

WebKit has already landed a fix for this issue, see https://webkit.org/b/176096

I've submitted a similar patch in Blink, see https://crrev.com/c/643106. It's currently under review and waiting for feedback from the Firefox team. Interoperability is an important issue for this case, so it'd be great to have some idea about any plan in Mozilla to work on this bug.

Thanks.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I think Mats or Ehsan may be the most appropriate people to take a look at it and see how feasible / etc. this is.

Mats, Ehsan, any chance you could take a look at this?

Comment 7

[(no longer active)]

In Gecko the bug isn't strictly related to floats actually, it can happen at any time the mouse cursor ends up on the canvas frame as far as I can tell.  This can happen sooner with floats (because they're parented by the canvas frame) but if you modify the test case and remove float: left from the innermost div and move the mouse cursor out of all of the boxes the bug still reproduces.

I debugged this a bit.  The problem seems to come from the fact that nsFrameSelection::HandleDrag receives events for the frame that the user is currently dragging the mouse cursor on, which for example can be the block frame for the body element.  In that case we try to extend the selection to (body, 3) in the test case here for example <https://searchfox.org/mozilla-central/rev/f6dc0e40b51a37c34e1683865395e72e7fca592c/layout/generic/nsFrameSelection.cpp#1486> and since the selection's anchor point is inside a text node in the text area, that creates the artifact in comment 0.

I don't have any good ideas for how we'd fix this, perhaps Mats would...

Comment 8

[Javier Fernandez]

Well, it's true that removing the float: left from the innermost div the bug still reproduces, but I think it's because of the absolute positioned parent.

Attached a new test case which reproduces the bug only if the innermost div is a float.

Comment 9

[Javier Fernandez]

Attachment: dragSelectionExtent1-1.html

Comment 10

[Javier Fernandez]

BTW, the test case without float but with the absolute positioned parent is also buggy in Chrome. The reason is that it implements a similar logic, in terms of HitTesting and Selection, for any out of flow box. Currently, I'm only addressing the float related issues, but I'm pretty sure there will be similar problems with positioned boxes.

Comment 11

[(no longer active)]

Indeed, you're right.  I actually noticed under the debugger that here for example <https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.cpp#511> the root node that we find is the absolutely positioned node but didn't realize this is an important factor in whether the bug occurs or not.

Comment 12

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #11)
> Indeed, you're right.  I actually noticed under the debugger that here for
> example
> <https://searchfox.org/mozilla-central/rev/
> 3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.
> cpp#511> the root node that we find is the absolutely positioned node but
> didn't realize this is an important factor in whether the bug occurs or not.

Err, my memory was wrong there.  We get the HTML element there.  I compared what happened in these two test cases under the debugger inside ConstrainFrameAndPointToAnchorSubtree().  In the successful case, ConstrainFrameAndPointToAnchorSubtree() gets called with the block frame for the HTML element, so content in <https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.cpp#518> is the HTML node, and contentRoot and anchorRoot are the same.  We take this path out of the function <https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.cpp#554> (even though the computed return value doesn't change.)

In the failing case, content in <https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.cpp#518> is the absolutely positioned DIV node, and contentRoot and anchorRoot are the HTML node, so we end up taking this early return <https://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/generic/nsFrameSelection.cpp#532>.

As a result, we return the block frame for the div in the failing case, and the block frame for the HTML node in the passing case.  This causes the content and offset returned from GetContentOffsetsFromPoint() to be different in the two cases.

Still unsure where the right fix would be for this.  Perhaps it is GetSelectionClosestFrame() that's not working correctly.  This is code that I'm not deeply familiar with really.

Comment 13

[Javier Fernandez]

Considering that WebKit [1] has already landed a fix for this issue and that Blink [2] has a patch ready, pending of review and feedback from Gecko, it'd be great to have some info about the intentions of fix it, eventually. I'm worried about interoperability here; there are more issues like this one that I'm willing to fix in the coming months, mainly related to Selection and Floats, so it'd be great to get some kind of agreement on the expected behavior and willingness to implement it.

Comment 14

[Javier Fernandez]

forgot to add the links to the patches:

1- https://bugs.webkit.org/show_bug.cgi?id=176096
2- https://chromium-review.googlesource.com/c/chromium/src/+/643106