1232322 - Double-clicking on a word can select two words

Status: NEW

(Core :: DOM: Selection, defect)

Description

[Vincent Lefevre]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Iceweasel/38.4.0
Build ID: 20151103222033

Steps to reproduce:

1. Open https://bugzilla.mozilla.org/show_bug.cgi?id=1119526 (or any other bug page).
2. In some comment, double click on the year. For instance, "2015" in the line:
      Vincent Lefevre 2015-01-08 22:28:32 UTC


Actual results:

The year and the word before are both selected, e.g. "Lefevre 2015" (what is between the double-quotes has been pasted, thus directly comes from the selection).


Expected results:

Only the year should have been selected.

Note that I have:
  layout.word_select.eat_space_to_next_word = false
  layout.word_select.stop_at_punctuation = true
(this is the default).

This bug also occurs with Nightly in safe-mode and a fresh profile.

Comment 1

[Alice0775 White]

Attachment: index.html

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=eaab632b48a3&tochange=eb3e5d23f987

Regressed by:3f46962ff0ce	Alexander Surkov — Bug 853361 - moving by words is inconsistent, r=roc

Comment 2

[alexander :surkov (:asurkov)]

I'm not sure I'm the best person to answer the question, since I'm a guest of this module. Could we get somebody who knows it to take a look please?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: test11.html

Minimized testcase. Clicking on "kitty" shouldn't select "hello", but does.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The bug is in nsTextFrame::PeekOffsetWord as it scans backwards looking for the previous word boundary.
-- It starts scanning in the middle of the second textnode.
-- It reaches the point immediately after the space at the start of the second textnode. This is deemed not a valid place to stop, because the space has been collapsed away by CSS (cIter.NextCluster() skips it and returns false because we run off the beginning of the frame).
-- Then we proceed into the first textnode. The point after the space at the end of the textnode is considered not a valid stopping-place because there's no word-break opportunity there (word-break opportunities are considered only at the boundary between whitespace and non-whitespace).
-- The point before the space at the end of the first textnode is rejected because of the check added in bug 853361: aWordSelectEatSpace is false, and the next character is not whitespace. Before bug 853361, though, we would have incorrectly selected the space before "kitty", so there was already a bug.

I think the underlying problem here is that the space with the word-break we want to stop at has been collapsed away, so the word-break is effectively ignored. We should never just ignore a word-break.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33e3efd4dfd

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1232322. Allow caret movement to stop at break opportunities induced by collapsed whitespace. r=mats

Review commit: https://reviewboard.mozilla.org/r/37483/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37483/

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 8725422 [details]
MozReview Request: Bug 1232322. Allow caret movement to stop at break opportunities induced by collapsed whitespace. r=mats

https://reviewboard.mozilla.org/r/37483/#review34175

This seems like the right approach.  It looks like it made a few tests fail though:
accessible/tests/mochitest/hittest/test_shadowroot.html fails accross platforms
layout/generic/test/test_movement_by_words.html fails on OSX10.10

Comment 9

[Vincent Lefevre]

Attachment: Another testcase, with more spaces

I've attached another similar testcase (same initial example, coming from a different Bugzilla) with more spaces, so that the effect can be even worse:
* With Firefox 52.9.0, lots of spaces, including newlines, are also in the selection.
* With Firefox Nightly 61.0.1, the two words are still selected with spaces, but any sequence of whitespace is collapsed into a single space character (this is better, in particular, potentially dangerous newlines are no longer in the selection, but still incorrect).

Comment 10

[Pascal Chevrel:pascalc]

Removing the regression keyword since it regressed with Firefox 38 and at this point, this is no longer a regression but regular bug work.

Comment 11

[John Cardinal]

I see an issue that may be due to the same cause or may be different. If I should create a new bug, please let me know.

I have a table with several rows, two columns each. where the second column has an absolutely-positioned BUTTON on the right. The BUTTON has no text, it relies on a background image. Here's example markup for the table:

<table>
	<tbody>
		<tr>
			<th>Label one</th>
			<td>Value one<button class="copy-value" tabindex="0"></button></td>
		</tr>
		<tr>
			<th>Label two</th>
			<td>Value two</td>
		</tr>
	</tbody>
</table>

Here's the CSS:

td, th {
	border: 1px solid #888;
	min-width: 6em;
	padding-right: 24px;
	position: relative;
}

.copy-value {
	height: 16px;
	position: absolute;
	right: 2px;
	top: 4px;
	width: 16px;
}

Result:

Double-clicking the "one" in the "Value one" column selects that word as well as the first word in the first column of the next row, the word "Label" in "Label two".

Expected result:

Only the "one" in "Value one" selected.

If I remove the BUTTON element, or set it to "display: none", the selection issue goes away. If the BUTTON element does not have "position: absolute", that also avoids the issue.

Comment 12

[Vincent Lefevre]

Attachment: double-click-3.html

I've attached this testcase, a bit simplified. I can reproduce the bug with Firefox 101.0.1 under Debian/unstable.

Comment 13

[Vincent Lefevre]

This issue is still present in FF 115.8.0esr, but I cannot reproduce it in FF 123.0.1 (using the double-click-3.html testcase attached in comment #12).

Comment 14

[John Cardinal]

In FF 123.0.1 under Win11, I can no longer reproduce the issue with the symptoms I reported, ("I have a table with several rows, two columns each...").