New issue
Advanced search Search tips

Issue 888424 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Caret in contenteditable not in proper position

Reported by helge...@gmail.com, Sep 24

Issue description

Chrome Version       : 69.0.3497.100
OS Version: 10.0
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari:
    Firefox: OK
    IE/Edge: OK

What steps will reproduce the problem?
1. Open test case
2. Click empty space below the first .pagebreak element
3. Watch caret get placed above said .pagebreak

What is the expected result?
Caret is placed at the right-most position of the nearest line the user has clicked.

What happens instead of that?
Caret is placed above other element.

Please provide any additional information below. Attach a screenshot if
possible.
I have a screencast illustrating the problematic behaviour: https://www.screencast.com/t/guOzXPHok
And a jsfiddle: https://jsfiddle.net/q4pu37dn/16/show
I also did install older versions of Chrome and back on version 64 and older everything seems fine.

UserAgentString: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36



 
jsbin.zaxujej.2.html
1.3 KB View Download
Components: Blink>Editing>Selection
Labels: Needs-Triage-M69
Cc: vamshi.kommuri@chromium.org
Labels: -Type-Bug Needs-Bisect Triaged-ET Type-Bug-Regression
Status: Untriaged (was: Unconfirmed)
Thanks for filing the issue!

Able to reproduce the issue on reported chrome version 69.0.3497.100 and the issue isn't seen on M60(60.0.3112.0), hence considering it as Regression and tentatively marking it as Untriaged. Will update the Bisect info soon.
Labels: -Pri-3 -Needs-Bisect hasbisect-per-revision RegressedIn-65 Target-70 Target-71 M-71 FoundIn-71 FoundIn-70 Target-69 FoundIn-69 OS-Linux OS-Mac Pri-1
Owner: jfernan...@igalia.com
Status: Assigned (was: Untriaged)
++Issue is seen on latest beta 70.0.3538.22 and canary 71.0.3560.0 using Windows 10, Ubuntu 14.04 and Mac 10.13.1

Bisect Information:
-------------------
Good Build: 65.0.3292.0
Bad Build:  65.0.3293.0

You are probably looking for a change made after 523409 (known good), but no later than 523410 (first known bad).
CHANGELOG URL:  https://chromium.googlesource.com/chromium/src/+log/a22b9873c5fa353ad274e5408b0f8d8f5c54781d..ebf3a5282adcb4eaa862516508517d57ae052009
Suspecting: https://chromium.googlesource.com/chromium/src/+/ebf3a5282adcb4eaa862516508517d57ae052009
Review URL: https://chromium-review.googlesource.com/643106

@Javier Fernandez: Please help in assigning it to right owner if this is not related to your change.

Thanks!
I'll take a look.
Status: Started (was: Assigned)
I can confirm the bug and the results of the bisect; the identified change is indeed the cause of the regression. 

I'm working on on a solution.
There is still a problem with the caret with multicolumns and contenteditable. Using with multiple paragraphs the caret is missing always on the last paragraph of the middle columns if the paragraph flow to the next column.
The easiest solution would be to revert the change that caused the regression. I've got already approval from 2 reviewers in https://crrev.com/c/1271102. 

However, I think I can try to find a solution to fix the issue and avoid the revert, which I think it a step in the right direction to improve the selection with floats. I'm already working on it and got a promising approach, but I'd need more time to define additional regressions tests.
Could you see the problem with the multicolumns too?
Sounds great Javier! I am excited with the progress, checking every day for any news :)

Comment 12 Deleted

Link correction for the caret multicolumn bug: https://jsfiddle.net/a7zLdh9t/4/
In addition to the revert, I've implemented a change that fixes the float related case initially reported. 

> Could you see the problem with the multicolumns too?

I'm not sure about this case, though. If I understood it correctly, it's also reproducible after reverting the change identified as culprit by the bisect. So, either it's a bug which was already present before the change, or it's not a bug at all.

There is a bug for sure. If a text flow to the next column within the same <p> for example. The caret is invisible at the paragraph in the initial column.
I agree that your multicolumns example looks buggy. But still, I don't think it is the same bug we are talking about here in this issue. In your example when characters are typed they end up where they should and the only real issue is that the caret is not visible. In the issue I have reported here the caret is actually visible but in wrong position and typed characters will also end up in the wrong position. I don't see how these two bugs are the same...
I agree with comment #16. If there is a bug, it'd be one totally different to the issue originally reported. I recommend to file a new issue against the multi column component.
Just to be clear... i know is a different bug. A new ticket is the right thing to do.
Project Member

Comment 19 by bugdroid1@chromium.org, Nov 19

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

commit b726621f5d8dd271e0b5c238d3770245b6ab6047
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Mon Nov 19 12:24:17 2018

Revert "Consider floats as HitTest candidate for selection"

This reverts commit ebf3a5282adcb4eaa862516508517d57ae052009.

Reason for revert: This change caused a regression in how caret is positioned in contenteditable areas with floats or multicolumns. 

Original change's description:
> Consider floats as HitTest candidate for selection
> 
> Selection with floats has a weird and unpredictable behavior. The root
> cause is that floats are not considered valid HitTest candidates.
> 
> There are many other cases where float elements are ignored when
> traversing the tree looking for the closes element to a specific
> LayoutPoint.
> 
> This patch address just one of this cases, where floats are children
> of a in-flow block-level box.
> 
> Bug: 758526
> Change-Id: I918af3ee21aa1070fa03a9fe1073205cc0a57300
> Reviewed-on: https://chromium-review.googlesource.com/643106
> Reviewed-by: Philip Rogers (OOO) <pdr@chromium.org>
> Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#523410}

TBR=yosin@chromium.org,pdr@chromium.org,jfernandez@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 758526,  888424 
Change-Id: Iba0f1641eeae63a52ef914b334cb302e69b7e7d9
Reviewed-on: https://chromium-review.googlesource.com/c/1271102
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#609248}
[delete] https://crrev.com/9ca81adb5affc813264c45c26b98cfcc96576a14/third_party/WebKit/LayoutTests/editing/selection/select-out-of-floated-non-editable.html
[modify] https://crrev.com/b726621f5d8dd271e0b5c238d3770245b6ab6047/third_party/blink/renderer/core/layout/layout_block.cc

Status: Fixed (was: Started)
This issue should be FIXED now.
Labels: TE-Verified-M72 TE-Verified-72.0.3616.0
Verified the fix on Mac 10.14.1, Windows-10 and Ubuntu 14.04 using Chrome version 72.0.3616.0 as per the comment #0.
Attaching screen cast for reference.
Observed that caret is placed where ever user is clicking.
Hence, the fix is working as expected. 
Adding the verified labels.
Note: Able to reproduce the issue on chrome version with out fix.

Thanks...!!
888424.mp4
618 KB View Download
I can confirm the fix works in newest Chromium for my jsfiddle example. Thank you very much! :)

Sign in to add a comment