[Webkit-unassigned] [Bug 33226] Indent inside li with br's changes order and creates new lists

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #208513|review?                     |review-
               Flag|                            |




--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org>  2013-08-20 22:55:35 PST ---
(From update of attachment 208513)
View in context: https://bugs.webkit.org/attachment.cgi?id=208513&action=review

> Source/WebCore/ChangeLog:15
> +        Place the cursor before or after the <br> tag,inside the <li> element
> +        and apply the indent command ,applied element always moves to beginning
> +        of the list item.
> +        <br> tag is present inside the <li> tag , text range is split based on
> +        the <br> element present in the <li> element, When we apply  indent to
> +        particular text range of the <li> element ,we need to split the node based
> +        on the previous and  next element , to place the newly created element of 
> +        <ul><li>Text</li></ul>,so added new logic to handle this scenario.

Comma is all messed up in this description.  Each comma should appear immediately after the preceding letters and should be followed by exactly one space.
e.g. hello, world.  Not hello , world or hello,world.
Also, I don't really get what you mean in the second paragraph. Please revise it.

> Source/WebCore/ChangeLog:18
> +        Test: editing/execCommand/indent_on_li_element_containing_br_text.html

Please use - instead of _ to separate words.

> Source/WebCore/editing/IndentOutdentCommand.cpp:-73
> -    // FIXME: previousElementSibling does not ignore non-rendered content like <span></span>.  Should we?

Why are we removing this comment?

> Source/WebCore/editing/IndentOutdentCommand.cpp:74
> +    Node* beforeLastNode = lastNodeInSelectedParagraph->previousSibling();
> +    Node* afterLastNode = lastNodeInSelectedParagraph->nextSibling();

Why are we getting previous and next siblings?  They may not be even visible. e.g. collapsed whitespaces.

> Source/WebCore/editing/IndentOutdentCommand.cpp:78
> +    if (!beforeLastNode && afterLastNode)
> +        insertNodeBefore(newList, selectedListItem);

I don't understand why we need to adjust where we insert a list item based on the content of li.
It looks like you're simply trying to work around a bug in moveParagraphWithClones,

> Source/WebCore/editing/IndentOutdentCommand.cpp:80
> +        splitElement(selectedListItem, lastNodeInSelectedParagraph);

lastNodeInSelectedParagraph is not necessary a child of selectedListItem. e.g. <li><span><br></span></li>
r- because of this wrong assumption.

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