Bug 19403 - ASSERT in InsertIntoTextNodeCommand::InsertIntoTextNodeCommand during RemoveFormat call
Summary: ASSERT in InsertIntoTextNodeCommand::InsertIntoTextNodeCommand during RemoveF...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-05 17:29 PDT by Justin Garcia
Modified: 2008-06-05 21:46 PDT (History)
1 user (show)

See Also:

Attachments
test case (239 bytes, text/html)
2008-06-05 17:30 PDT, Justin Garcia 		no flags Details
patch (2.71 KB, patch)
2008-06-05 20:56 PDT, Justin Garcia 		eric: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Garcia 2008-06-05 17:29:50 PDT 
Run the attached test case in a debug build.

ASSERT(!text.isEmpty()) inside InsertIntoTextNodeCommand::InsertIntoTextNodeCommand fires because RemoveFormat calls input with an empty String.  plainText() is returning an empty string for a Range selection for some reason...
Comment 1 Justin Garcia 2008-06-05 17:30:27 PDT 
Created attachment 21516 [details]
test case
Comment 2 Justin Garcia 2008-06-05 20:56:04 PDT 
Created attachment 21519 [details]
patch
Comment 3 Eric Seidel (no email) 2008-06-05 21:32:18 PDT 
Comment on attachment 21519 [details]
patch

Looks good.  The if (string.isEmpty()) should have a comment before it explaning "why", since I'm not sure it will be clear to either of us why it's there in 9 months. :)


If you strongly disagree I'm fine w/ you to landing w/o comment.  I don't need to see your final patch again either way. :)
Comment 4 Justin Garcia 2008-06-05 21:46:04 PDT 
(In reply to comment #3)
> (From update of attachment 21519 [details] [edit])
> Looks good.  The if (string.isEmpty()) should have a comment before it
> explaning "why", since I'm not sure it will be clear to either of us why it's
> there in 9 months. :)

You're right.  Fixed that.  http://trac.webkit.org/changeset/34399