Closed Bug 1327902 Opened 7 years ago Closed 7 years ago

Text selection by drag-n-drop is canceled if input is there (on https://vk.com/)

Categories

(Core :: DOM: Selection, defect)

Product:
Core
Component:
DOM: Selection
Version:
47 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: arni2033, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: regression, testcase)

Attachments

(3 files)

testcase 1 - text selection is canceled if I input is in selection.htm
278 bytes, text/html
Details
Set the range boundary point outside (before) the node if it HasIndependentSelection().
1.60 KB, patch
smaug
: review+
jcristau
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Test selecting over a user-select:none node next to an <input>.
1.91 KB, patch
Details | Diff | Splinter Review 
>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Copy string "asdf" to clipboard. Open attached "testcase 1"
2. Move mouse between "H" and "e" in string "Hell", hold left mouse button to start selection
3. Move mouse between "l" and "l" in string "Hell", then between "a" and "i" in string "pain"
4.(bonus) Release left mouse button. Press Ctrl+C, click in input, press Ctrl+V, press Ctrl+A


STR_2:  (original)
1. Copy string "asdf" to clipboard. Open  https://vk.com/wall-7736865_418396
2. Move mouse to the bottom of empy area, hold left mouse button to start selection
3. Move mouse to the text above, then to the buttons on the left, then to the text above

AR:
 In step 3 selection stops updating. In Step 4 wrong test is pasted in <input>; text in <input> is highlighted with blue simultaneously with text in the line above.
 
ER:  
 In Step 3 selection should update to contain second line. In Step 4 (only!) highlighted text should
 be copied to clipboard. Text in <input> shouldn't be highlighted simultaneously with text on page.


Notes:
1) See another clone of bug 666618 before you start finding regression window - it may be helpful
2) You can also use "data:" url [1], which is identical to testcase 1. HTML is shown below.

> [1] data:text/html,<!DOCTYPE html><style class="created-by-arni2033">.no_select {-webkit-user-select:none;-moz-user-select:none;-ms-user-select:none;user-select:none}</style><div>Hello world</div><div class="no_select">full</div><input placeholder="of"><div>pain!</div>

data:text/html,
<!DOCTYPE html>
<style class="created-by-arni2033">
.no_select {
	-webkit-user-select:none;
	-moz-user-select:none;
	-ms-user-select:none;
	user-select:none
}
</style>
<div>Hello world</div>
<div class="no_select">full</div>
<input placeholder="of">
<div>pain!</div>
No longer blocks: 1277113
Component: Untriaged → Selection
Product: Firefox → Core
Narrowed inbound regression window from [28c3f022, 3c7cf3cc] (3 revisions) to [db0f2e93, 3c7cf3cc] (2 revisions) (~1 steps left)
Oh noes, no (more) inbound revisions :(
Last good revision: db0f2e9327aff428b9258db23ae0508f6a0ad861
First bad revision: 3c7cf3cca248c91c9f674e513ed0a797d3da7374
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=db0f2e9327aff428b9258db23ae0508f6a0ad861&tochange=3c7cf3cca248c91c9f674e513ed0a797d3da7374

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1216001

Mats, can you take a look at this?
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
Flags: needinfo?(mats)
Keywords: regressionwindow-wanted → regression
Version: Trunk → 47 Branch
So, what happens is nsRange::ExcludeNonSelectableNodes is called
with a range spanning from "Hello" to "pain" and it splits it up
in two ranges - the second with a start boundary point: <input> / 0.
The problem with that is it's *inside* the text control, which is
a node with HasIndependentSelection() true.  So when we get a call
to nsFrameSelection::HandleDrag for the "pain" text frame, we call
nsINode::GetSelectionRootContent on that anchor we get a different
root compared to the root for the "pain" text node, so we end up
calling Selection::Extend with the anon <div> inside text control...
Assignee: nobody → mats
Blocks: 1216001
Has STR: --- → yes
Flags: needinfo?(mats)
Keywords: testcase
OS: Unspecified → All
Hardware: Unspecified → All
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID 	20170104133516

I have tested the try build from comment 5 and the issue is no longer reproducible.
Testing was performed on Windows 10 x64, Windows 7 x64, Mac OS 10.11 and Ubuntu 14.04 x64.
Attachment #8823868 - Flags: review?(bugs) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/944f680d7707
Set the range boundary point outside (before) the node if it HasIndependentSelection().  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f86d0941919c
Test selecting over a user-select:none node next to an <input>.
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/944f680d7707
https://hg.mozilla.org/mozilla-central/rev/f86d0941919c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Ciprian, can you please verify that this is fixed on Nightly? Mats, I'm not sure this is worth backporting to Beta at this point, but what are your thoughts about Aurora at least?
status-firefox50: affected → wontfix
Flags: needinfo?(mats)
Flags: needinfo?(ciprian.muresan)
-moz-user-select:none seems like an edge case so probably not worth it for Beta.
The patch is low risk though so OK for Aurora IMO.
Flags: needinfo?(mats)
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID 	20170109030209

I have retested the issue on the latest Nightly and it is no longer reproducible.
As before, testing was performed on Windows 10 x64, Windows 7 x64, Mac OS 10.11 and Ubuntu 14.04 x64.
Flags: needinfo?(ciprian.muresan)
Comment on attachment 8823868 [details] [diff] [review]
Set the range boundary point outside (before) the node if it HasIndependentSelection().

Approval Request Comment
[Feature/Bug causing the regression]: bug 1216001
[User impact if declined]: text selection issues in some situations
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: already done
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not really, just rearranging code
[Why is the change risky/not risky?]: see above
[String changes made/needed]: none
Attachment #8823868 - Flags: approval-mozilla-aurora?
Comment on attachment 8823868 [details] [diff] [review]
Set the range boundary point outside (before) the node if it HasIndependentSelection().

fix for text selection with non-selectable nodes, take in aurora52
Attachment #8823868 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: