New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 683776 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Fallback content in slot causes backward selection to jump around

Project Member Reported by dominicc@chromium.org, Jan 23 2017

Issue description

This was reported by a user in the Polymer 2-0-preview slack channel.

Chrome Version: 55.0.2883.87 (Official Build) (64-bit)
OS: Linux

What steps will reproduce the problem?
(1) Open the attachment
(2) Select from "three" backwards to "one"

What is the expected result?

The selection extends backwards.

What happens instead?

The selection jumps around over line two.
 
repro.html
516 bytes View Download
chromebug.gif
35.0 KB View Download

Comment 1 by yosin@chromium.org, Jan 30 2017

Cc: -yosin@chromium.org
Status: Available (was: Untriaged)
It seems backward flat tree traversal with fallback content does wrong.
Or comparePositions()[1]

[1] core/editing/EditingUtilities.cpp
template <typename Traversal>
static int comparePositions(Node* containerA,
                            int offsetA,
                            Node* containerB,
                            int offsetB,
                            bool* disconnected) {

Comment 2 by hayato@chromium.org, Jan 30 2017

Components: Blink>DOM>ShadowDOM

Comment 3 by kochi@chromium.org, Mar 15 2017

Owner: kochi@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 16 2017

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

commit b1c7a1d78f6c79f18f1686014a81b86abfb38806
Author: kochi <kochi@chromium.org>
Date: Thu Mar 16 12:03:50 2017

Handle fallback content case in FlatTreeTraversal::traverseSiblings()

FlatTreeTraversal::nextSibling()/previousSibling() didn't handle
fallback content (nodes in <slot> where nothing is distributed).

This CL adds fallback content handling and a unit test case.

BUG= 683776 

Review-Url: https://codereview.chromium.org/2749033006
Cr-Commit-Position: refs/heads/master@{#457399}

[modify] https://crrev.com/b1c7a1d78f6c79f18f1686014a81b86abfb38806/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp
[modify] https://crrev.com/b1c7a1d78f6c79f18f1686014a81b86abfb38806/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp

Comment 5 by kochi@chromium.org, Mar 17 2017

Status: Fixed (was: Started)

Comment 6 by kochi@chromium.org, Mar 17 2017

Labels: Merge-Request-58 M-58
Project Member

Comment 7 by sheriffbot@chromium.org, Mar 17 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 17 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8e8f038e206c970e47d608a8181ef8f281d5c17

commit b8e8f038e206c970e47d608a8181ef8f281d5c17
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Fri Mar 17 02:01:33 2017

Handle fallback content case in FlatTreeTraversal::traverseSiblings()

FlatTreeTraversal::nextSibling()/previousSibling() didn't handle
fallback content (nodes in <slot> where nothing is distributed).

This CL adds fallback content handling and a unit test case.

BUG= 683776 

Review-Url: https://codereview.chromium.org/2749033006
Cr-Commit-Position: refs/heads/master@{#457399}
(cherry picked from commit b1c7a1d78f6c79f18f1686014a81b86abfb38806)

Review-Url: https://codereview.chromium.org/2756083002 .
Cr-Commit-Position: refs/branch-heads/3029@{#253}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/b8e8f038e206c970e47d608a8181ef8f281d5c17/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp
[modify] https://crrev.com/b8e8f038e206c970e47d608a8181ef8f281d5c17/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp

Cc: rbasuvula@chromium.org
Labels: TE-Verified-M58 TE-Verified-58.0.3029.33
Tested the issue on Windows-10, Ubuntu 14.04,Mac OS 10.12.3 and Chrome OS using chrome latest Beta M58-58.0.3029.33 and chrome OS latest build #58.0.3029.31 by following steps mentioned in the original comment. Observed that text selection working as expected. Hence adding TE-Verified label.

Please view the screen cast for reference.

Thank you!
683776.ogv
1.5 MB View Download

Sign in to add a comment