Bug 48658

Summary: Deleting contenteditable text to the left of a non-contenteditable span inserts unnecessary placeholder <br/>
Product: WebKit Reporter: Benjamin (Ben) Kalman <kalman>
Component: New BugsAssignee: Benjamin (Ben) Kalman <kalman>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, commit-queue, darin, enrica, kalman, noel.gordon, pnormand, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 48037    
Attachments:
Description Flags
Patch
none
Simple repro
none
Patch
none
Patch none

Benjamin (Ben) Kalman
Reported 2010-10-29 12:17:49 PDT
Deleting contenteditable text to the left of a non-contenteditable span inserts unnecessary placeholder <br/>
Attachments
Patch (13.56 KB, patch)
2010-10-29 12:21 PDT, Benjamin (Ben) Kalman 		no flags
DetailsFormatted DiffDiff
Simple repro (74 bytes, text/html)
2010-10-29 12:22 PDT, Benjamin (Ben) Kalman 		no flags
Details
Patch (12.24 KB, patch)
2010-10-29 12:30 PDT, Benjamin (Ben) Kalman 		no flags
DetailsFormatted DiffDiff
Patch (13.36 KB, patch)
2010-10-29 13:46 PDT, Benjamin (Ben) Kalman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Benjamin (Ben) Kalman
Comment 1 2010-10-29 12:21:23 PDT
Created attachment 72367 [details] Patch
Benjamin (Ben) Kalman
Comment 2 2010-10-29 12:22:41 PDT
Created attachment 72368 [details] Simple repro
Benjamin (Ben) Kalman
Comment 3 2010-10-29 12:23:18 PDT
Repro attached -- highlight "hello " (i.e. including space) and delete.
Benjamin (Ben) Kalman
Comment 4 2010-10-29 12:25:33 PDT
Comment on attachment 72367 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72367&action=review > WebCore/WebCore.exp.in:420 > +__ZN7WebCore16isEndOfParagraphERKNS_15VisiblePositionENS_8Position27EditingBoundaryCrossingRuleE fyi I tried deleting these but linking fails with Undefined symbols: "__ZN7WebCore16isEndOfParagraphERKNS_15VisiblePositionENS_8Position27EditingBoundaryCrossingRuleE", referenced from: -[WebFrame(WebPrivate) _smartInsertForString:replacingRange:beforeString:afterString:] in WebFrame.o "__ZN7WebCore18isStartOfParagraphERKNS_15VisiblePositionENS_8Position27EditingBoundaryCrossingRuleE", referenced from: -[WebFrame(WebPrivate) _smartInsertForString:replacingRange:beforeString:afterString:] in WebFrame.o
Benjamin (Ben) Kalman
Comment 5 2010-10-29 12:30:06 PDT
Created attachment 72371 [details] Patch
Ryosuke Niwa
Comment 6 2010-10-29 12:35:43 PDT
Comment on attachment 72371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72371&action=review LGTM. > LayoutTests/editing/deleting/5390681-expected.txt:-2 > This tests for a bug where expansion for smart delete would not consider editable boundaries. Only 'foo' should be deleted. You should see ' bar'. There is a bug: while the non-editable space isn't deleted, deletion inserts a placeholder when it shouldn't. > -| <br> You need to update the comment here.
Benjamin (Ben) Kalman
Comment 7 2010-10-29 13:46:21 PDT
Created attachment 72383 [details] Patch
Ryosuke Niwa
Comment 8 2010-10-29 14:03:03 PDT
Comment on attachment 72383 [details] Patch LGTM.
WebKit Commit Bot
Comment 9 2010-10-29 14:47:11 PDT
Comment on attachment 72383 [details] Patch Clearing flags on attachment: 72383 Committed r70932: <http://trac.webkit.org/changeset/70932>
WebKit Commit Bot
Comment 10 2010-10-29 14:47:18 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 11 2010-10-29 15:15:40 PDT
Normally to share an enum like this we’d move the enum into a header instead of including "Position.h" in a header like "visible_units.h".
Benjamin (Ben) Kalman
Comment 12 2010-11-01 13:20:29 PDT
Ok, happy to do that. What is the standard practice here?
Darin Adler
Comment 13 2010-11-03 12:38:05 PDT
(In reply to comment #12) > Ok, happy to do that. What is the standard practice here? Create the new header, add it to the build systems, put the enum in there, add includes to the appropriate source files.
Benjamin (Ben) Kalman
Comment 14 2010-11-16 16:02:45 PST
(In reply to comment #13) > (In reply to comment #12) > > Ok, happy to do that. What is the standard practice here? > > Create the new header, add it to the build systems, put the enum in there, add includes to the appropriate source files. https://bugs.webkit.org/show_bug.cgi?id=49630
Philippe Normand
Comment 15 2011-11-11 06:48:00 PST
editing/pasteboard/paste-text-events.html editing/pasteboard/copy-backslash-with-euc.html Started to fail on GTK since this patch landed it seems.
Philippe Normand
Comment 16 2011-11-11 08:02:19 PST
(In reply to comment #15) > editing/pasteboard/paste-text-events.html > editing/pasteboard/copy-backslash-with-euc.html > > Started to fail on GTK since this patch landed it seems. Heh. Nevermind!
Note You need to log in before you can comment on or make changes to this bug.