[Webkit-unassigned] [Bug 53409] InsertUnorderedList over a non-editable region and multiple lines enters an infinite loop

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53409





--- Comment #16 from Levi Weintraub <leviw at chromium.org>  2011-03-09 15:10:11 PST ---
(From update of attachment 85120)
View in context: https://bugs.webkit.org/attachment.cgi?id=85120&action=review

>> Source/WebCore/editing/CompositeEditCommand.cpp:952
>> +    VisiblePosition afterParagraph(endOfParagraphToMove.next(stayInEditableContent));
> 
> As I mentioned earlier, we should either change the argument type to enum or not do this change.

This isn't the same change you mentioned earlier, I reverted that one. The default value for both of these is "false" not "true" (hence the change) and the added boolean is for readability. (See thread "Bools are strictly worse than enums" on WebKit Dev for my rationale). If you'd rather see a magic value, I'm not dogmatic about this.

>> Source/WebCore/editing/InsertListCommand.cpp:69
>> +    return start == paragraphEnd && (!start.deepEquivalent().upstream(CanCrossEditingBoundary).anchorNode()->isContentEditable()
> 
> Why are we comparing with the end of paragraph? I'm also skeptical about the correctness of anchorNode() here.  Shouldn't it be containerNode?  Why do we care that node before/after the position is editable?

If the start and end of the paragraph are the same when staying within editable boundaries, but not when skipping over them, we're at a caret position we wish to ignore.

Since we're looking at content with mixed editability, it would be feasible for containerNode's editability to differ from the anchorNode's for before/after positions.

>> LayoutTests/editing/execCommand/insert-list-with-noneditable-content-expected.txt:5
>> +non-editable
> 
> Why do we have 4 paragraphs here?  Shouldn't we just have 2 paragraphs?  If we're only concerned with the crash and ignoring the correctness of output, then we should say that this test is testing that WebKit does not crash, and we should hide #test and print "PASS" indiscriminately.

We're matching FFX by treating regions of editable content in the same logical paragraph with non-editable content as different paragraphs. With that in mind, we should care about the output.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.