Double-clicking on a word can select two words

NEW
Unassigned

Status

()

3 years ago
5 days ago

People

(Reporter: vincent-moz, Unassigned)

Tracking

38 Branch
Points:
---

Firefox Tracking Flags

(firefox67 fix-optional)

Details

Attachments

(4 attachments)

(Reporter)

Description

3 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

3 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

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

Updated

3 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

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

Updated

8 days ago
status-firefox67: --- → affected

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

status-firefox67: affected → fix-optional
Keywords: regression
You need to log in before you can comment on or make changes to this bug.