New issue
Advanced search Search tips

Issue 748557 link

Starred by 2 users

Issue metadata

Status: Fixed
Merged: issue 17528
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Double-click on remaining text of ::first-letter doesn't work well

Project Member Reported by duga@google.com, Jul 25 2017

Issue description

Chrome Version       : 59.0.3071.115
OS Version: OS X 10.12.5
URLs (if applicable) : https://jsfiddle.net/9fxx5ntm/
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5: Partial FAIL, but not as severe
  Firefox 4.x: PASS

What steps will reproduce the problem?
1. Load the fiddle listed above.
2. Double click any word in the text before "ENDSPAN". 
2a. Alternatively, drag select over any of the text.

What is the expected result?

Properly selected word (or, if dragging, properly selected range).


What happens instead of that?

The selection appears to be shifted.


Screenshot attached. This is similar to https://bugs.chromium.org/p/chromium/issues/detail?id=17528 but that report is only for the first word, and only for dropping the first letter. This seems to break all text in the same element, and shifts the entire selection. I have observed cases where both the start and end of the selection are incorrect, though this example only shows the end as being incorrect. This is impacting Play Books, and will likely start hitting other ebook readers running on Chrome as publishers start adding ::first-letter styling to content.

This be entered as a restricted bug - I have no control over that, but I have no problem making it public.

UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36



 
select_fail.png
24.9 KB View Download
Labels: Needs-Milestone
Labels: -Needs-Milestone M-62 OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Windows 7, Mac 10.12.6 & Ubuntu 14.04 using latest chrome stable-60.0.3112.78 & Canary-62.0.3167.0.

Observed text selection issue & test drag issue in the above mentioned fiddle.
Same issue observed from M45 builds.Hence marking it as Untriaged to get more inputs from dev.
Please find the attached screencast for reference .

748557.mp4
779 KB View Download
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 26 2017

Labels: Hotlist-Google

Comment 4 by kojii@chromium.org, Jul 27 2017

Cc: garth@google.com

Comment 5 by kojii@chromium.org, Jul 27 2017

Cc: -garth@google.com
Labels: allpublic

Comment 6 by kojii@chromium.org, Jul 27 2017

Components: Blink>Editing>Selection

Comment 7 by yosin@chromium.org, Jul 28 2017

Mergedinto: 17528
Status: Duplicate (was: Untriaged)
first-letter doesn't work for
 - Select by mouse drag: Hit test is broken
 - Painting is broken: Set selection by Script: https://jsfiddle.net/9fxx5ntm/2/

Comment 8 by duga@google.com, Jul 28 2017

I am not sure why this was duped. The other bug is about selecting the first letter of an element styled with ::first-letter. This bug is about selecting everything else, which is also now broken, though in a different way. While I expect these may be in similar locations in the code (or even share a root cause), their behavior is quite different.

Comment 9 by yosin@chromium.org, Aug 17 2017

Status: Available (was: Duplicate)

Comment 10 by yosin@chromium.org, Aug 25 2017

Summary: Double-click on remaining text of ::first-letter doesn't work well (was: ::first-letter breaks selection of all text in the styled element)

Comment 11 by yosin@chromium.org, Aug 28 2017

It seems PreviousBoundary() called by StartOfWord() doesn't handle first-letter.
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed2864a8ee018ed22906a65e1ef6e1d1c1e3b37a

commit ed2864a8ee018ed22906a65e1ef6e1d1c1e3b37a
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Wed Aug 30 09:29:59 2017

Introduce test cases for SimplifiedBackwardsTextIterator

This patch introduces two test cases SimplifiedBackwardsTextIterator for
::first-letter pseudo element to record current behavior for improving code
health.

Bug:  748557 
Change-Id: I9334a19d6d3634a7b8f5a75b56dc76fa2add0d9f
Reviewed-on: https://chromium-review.googlesource.com/640730
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498409}
[modify] https://crrev.com/ed2864a8ee018ed22906a65e1ef6e1d1c1e3b37a/third_party/WebKit/Source/core/editing/iterators/SimplifiedBackwardsTextIteratorTest.cpp

Owner: yosin@chromium.org
Status: Started (was: Available)
In review: http://crrev.com/c/651393
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e484494aa3b54f5fd7477f0c6cf17eeb87751a63

commit e484494aa3b54f5fd7477f0c6cf17eeb87751a63
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Fri Sep 08 09:02:36 2017

Introduces test cases for StartWord()

This patch introduces test cases for |StartWord()| for improving code health.

Note: Results of ::first-letter are not good. Another patch will fix these
bad results.

Bug:  748557 
Change-Id: Id51c1ca10630b5ce74e590d73c342a1b7d95a8ee
Reviewed-on: https://chromium-review.googlesource.com/643007
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500542}
[modify] https://crrev.com/e484494aa3b54f5fd7477f0c6cf17eeb87751a63/third_party/WebKit/Source/core/editing/BUILD.gn
[add] https://crrev.com/e484494aa3b54f5fd7477f0c6cf17eeb87751a63/third_party/WebKit/Source/core/editing/VisibleUnitsWordTest.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54f391c73666902aa85961edfa165bd8a067c539

commit 54f391c73666902aa85961edfa165bd8a067c539
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Fri Sep 22 03:53:32 2017

Make SimplifiedBackwardsTextIterator::CopyTextTo() to work correctly for range ending in first-letter part

Before this patch |SimplifiedBackwardsTextIterator::CopyTextTo()| copies whole
first-letter part even if caller specified range ends within first-letter part.

This patch changes |SimplifiedBackwardsTextIterator::HandleFirstLetter()| to
use caller specified range end offset instead of end of first-letter part.

Bug:  748557 
Change-Id: I3828d226fe303c434364d2b7d50c77a236849200
Reviewed-on: https://chromium-review.googlesource.com/658166
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503651}
[modify] https://crrev.com/54f391c73666902aa85961edfa165bd8a067c539/third_party/WebKit/Source/core/editing/VisibleUnitsWordTest.cpp
[modify] https://crrev.com/54f391c73666902aa85961edfa165bd8a067c539/third_party/WebKit/Source/core/editing/iterators/SimplifiedBackwardsTextIterator.cpp
[modify] https://crrev.com/54f391c73666902aa85961edfa165bd8a067c539/third_party/WebKit/Source/core/editing/iterators/SimplifiedBackwardsTextIteratorTest.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/533744aa7772db7ede651fc9639d9598b3ee7420

commit 533744aa7772db7ede651fc9639d9598b3ee7420
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Mon Sep 25 09:24:07 2017

Make PreviousBoundary() to handle ::first-letter correctly

This patch makes |PreviousBoundary()| to handle ::first-letter correctly by
not using character offset in search buffer as DOM offset in |Text| node.

Bug:  748557 
Change-Id: If465e0ec7632b823bd3745aa58d85a29387357c6
Reviewed-on: https://chromium-review.googlesource.com/651393
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504014}
[modify] https://crrev.com/533744aa7772db7ede651fc9639d9598b3ee7420/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/533744aa7772db7ede651fc9639d9598b3ee7420/third_party/WebKit/Source/core/editing/VisibleUnitsWordTest.cpp

Comment 17 by yosin@chromium.org, Sep 28 2017

Status: Fixed (was: Started)

Sign in to add a comment