Double-clicking on a word can select two words

NEW
Unassigned

Status

()

Core
Selection
2 years ago
a year ago

People

(Reporter: Vincent Lefevre, Unassigned)

Tracking

({regression})

38 Branch
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

2 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

2 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

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

Updated

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

Updated

2 years ago
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(roc)

Comment 2

a year ago
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.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33e3efd4dfd
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)
Assignee: roc → nobody

Updated

a year ago
Duplicate of this bug: 1254379
You need to log in before you can comment on or make changes to this bug.