Double-clicking on a word can select two words

NEW
Unassigned

Status

()

defect
4 years ago
3 months ago

People

(Reporter: vincent-moz, Unassigned)

Tracking

38 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fix-optional)

Details

Attachments

(4 attachments)

Reporter

Description

4 years ago
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

4 years ago
Posted 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

Updated

4 years ago
Blocks: 853361
Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

3 years ago
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)
Posted 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)
Duplicate of this bug: 1254379
Reporter

Comment 9

10 months ago
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.

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