Open Bug 1232322 Opened 8 years ago Updated 14 days ago

Double-clicking on a word can select two words

Categories

(Core :: DOM: Selection, defect)

38 Branch
defect

Tracking

()

Tracking Status
firefox67 --- fix-optional

People

(Reporter: vincent-moz, Unassigned)

References

Details

(Keywords: regression)

Attachments

(5 files)

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.
Attached file 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
Blocks: 853361
Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(roc)
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?
Flags: needinfo?(surkov.alexander)
Attached file test11.html
Minimized testcase. Clicking on "kitty" shouldn't select "hello", but does.
Assignee: nobody → roc
Flags: needinfo?(roc)
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 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
Attachment #8725422 - Flags: review?(mats)
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).

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

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.

Attached file 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.

Severity: normal → S3

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).

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...").

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: