Placeholder BR following whitespace prevents to insert a space
Reported by
vic...@carerix.com,
Oct 25 2016
|
||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36 Steps to reproduce the problem: 1. create html page with next html: <div contenteditable="true"><br> </div> (or you can open fiddle example: https://jsfiddle.net/c715azfn/) 2. try to type some text with SPACES inside this div, f.e "Some text" What is the expected behavior? It is possible to enter spaces inside this DIV, for example if you will try to type "Some text" you will see "Some text" (with space between words) What went wrong? It is not possible to enter spaces inside this DIV, for example if you will try to type "Some text" you will see "Sometext" (without space between words) Did this work before? Yes Google Chrome version 53 Chrome version: 54.0.2840.71 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Shockwave Flash 23.0 r0 I think this is regression of this fix: https://codereview.chromium.org/2432083003 You can reproduce this issue in https://jsfiddle.net/c715azfn/
,
Oct 25 2016
Very annoying bug!
,
Oct 25 2016
,
Oct 26 2016
Able to reproduce the issue on windows 7, mac 10.11.4 and Linux Ubuntu 14.04 using Chrome stable 54.0.2840.71 ,dev 56.0.2896.3,beta 55.0.2883.21 But this is working fine on canary(56.0.2900.0). Seems the issue got fixed on later versions. Worked on reverse bisect and below is the info: Manual Bisect: ------------ First Good build:56.0.2897.0 Revision-426673 Last Bad build: 56.0.2896.0 Revision-426358 Bisect Tool Info: ---------------- https://chromium.googlesource.com/chromium/src/+log/dfb8322ccb6687fa5fd64029e9746049c12e06e1..952d55918cdb455223cabf647cef859486a37b0f suspect: ------- https://chromium.googlesource.com/chromium/src/+/952d55918cdb455223cabf647cef859486a37b0f Review-Url: ---------- https://chromiumcodereview.appspot.com/2432083003 joone.hur@ please help us to reassign this issue to a right owner if not with respect to this change.
,
Oct 26 2016
,
Oct 26 2016
This bug is reproducible in 56.0.2897.0, but it was fixed in ToT.
,
Oct 27 2016
Do you know what CL caused the regression? Was it first broken in Chrome 54 as reported by the person that filed the bug? If so that we should at least consider merging your fix into the 55 branch.
,
Oct 27 2016
,
Oct 27 2016
Here is the regression: https://codereview.chromium.org/2175163004 and the fix: ttps://codereview.chromium.org/2432083003
,
Oct 27 2016
,
Oct 28 2016
Right, so this regressed in Chrome 54, which only just hit stable, and you've fixed it for Chrome 56. I don't understand how bad this bug is (apparently it's not ALL contenteditable that's broken). I assume it's not SO bad that we'd consider merging a fix to Chrome 54. But we should at least serious consider merging a fix to M55. In general it's not good enough to just fix regressions on tip-of-tree if they've already made it out to a release. Yosi, WDYT? Worth merging to 55?
,
Oct 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f9400ff295d15df932a5ffc903e4ab42163b1b4d commit f9400ff295d15df932a5ffc903e4ab42163b1b4d Author: joone.hur <joone.hur@intel.com> Date: Mon Oct 31 02:31:54 2016 Add a test case for bug 659109 <div contenteditable="true"><br> </div> When we run the above example with M54/55, mutiple spaces can't be added under <div> because two plain spaces are collapsed into one space while typing. This is a regression caused by https://codereview.chromium.org/2175163004. It was fixed in M56: https://codereview.chromium.org/2432083003. This test case is added to reproduce the problem mentioned in the bug report. BUG= 659109 TEST=editing/inserting/insert-space.html Review-Url: https://codereview.chromium.org/2451423003 Cr-Commit-Position: refs/heads/master@{#428639} [modify] https://crrev.com/f9400ff295d15df932a5ffc903e4ab42163b1b4d/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html
,
Oct 31 2016
**** Bulk edit - please ignore if not applicable **** A friendly reminder that M55 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion). Thank you!
,
Oct 31 2016
rbyers@ we had better fix this problem in M55.
,
Nov 1 2016
yosin@/joone.hur@intel.com, please reply to comment #11.
,
Nov 2 2016
Sorry for late response. I agree this is annoying but it isn't serious since placeholder BR following whitespaces is rare. I think <div><br> </div> is a kind of authoring error, it is better to omit such redundant whitespace from the page. I agree to merge into M55 since it is just one line change, adding one codition to check a space. Note: <div contenteditable><br></div> and <div contenteditable></div> are equivalent.
,
Nov 2 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Nov 2 2016
Merged into M55: https://codereview.chromium.org/2473633002
,
Nov 2 2016
Issue 661375 has been merged into this issue.
,
Feb 7 2017
I can still reproduce this in 57.0.2987.21 beta. In particular the scenario we logged with issue 661375 where the <br /> is inside a nested element of the contenteditable, rather than just being the entire content of it: https://jsfiddle.net/x943naf6/6/
,
Feb 8 2017
Can this issue be reopened or should I log a new one?
,
Feb 8 2017
Hi Andrew, I'm looking into this problem.
,
Feb 9 2017
Andrew, can you open a new bug for this?
,
Feb 9 2017
I'm happy to do that, but it'll be an almost literal duplicate of issue 661375 . Can that issue be unmerged from this one?
,
Feb 9 2017
I need to ask yosin@ if we could reopen this bug. Anyway, issue 661375 is exactly the same issue you reported, but the root cause is slightly different from this bug. Here is a simple test case: <div style="border: 1px dashed black" class="edit" contenteditable="true"><br> </div> https://jsfiddle.net/cubix/fghc7oLv/ We need to check all kinds of white space including enter before inserting a space. Currently, we only check space.
,
Feb 9 2017
Agreed, and confirmed in my table case as well. It seems 661375 was marked as duplicate incorrectly. I don't mind opening a new one if it can't be unmerged.
,
Feb 9 2017
I unduped issue 661375 for you, hope that's OK yosin!
,
Feb 9 2017
Thanks! |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by vic...@carerix.com
, Oct 25 2016