Double-clicking on a word can select two words

NEW
Unassigned

Status

()

3 years ago
6 months ago

People

(Reporter: vincent-moz, Unassigned)

Tracking

({regression})

38 Branch
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

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
Created attachment 8698113 [details]
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)
Created attachment 8725025 [details]
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.
Created attachment 8725422 [details]
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/
Attachment #8725422 - Flags: review?(mats)
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

6 months ago
Created attachment 8992977 [details]
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).
You need to log in before you can comment on or make changes to this bug.