[Webkit-unassigned] [Bug 32131] WebCore::canHaveChildrenForEditing ReadAV at NULL (cd05b3b20e0f4c6b3afe5d165a1798aa)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 15 12:40:33 PST 2010


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





--- Comment #6 from Enrica Casucci <enrica at apple.com>  2010-01-15 12:40:32 PST ---
(From update of attachment 46645)
> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index 52b75bc..5ebd638 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,14 @@
> +2010-01-14  Tony Chang  <tony at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Editing crash when inserting a list.
> +        https://bugs.webkit.org/show_bug.cgi?id=32131
> +
> +        * editing/execCommand/19087-expected.txt:
> +        * editing/execCommand/insert-ordered-list-expected.txt: Added.
> +        * editing/execCommand/insert-ordered-list.html: Added.
> +
>  2010-01-13  Kent Hansen  <kent.hansen at nokia.com>
>  
>          Reviewed by Oliver Hunt.
> diff --git a/LayoutTests/editing/execCommand/19087-expected.txt b/LayoutTests/editing/execCommand/19087-expected.txt
> index 3211e80..391e821 100644
> --- a/LayoutTests/editing/execCommand/19087-expected.txt
> +++ b/LayoutTests/editing/execCommand/19087-expected.txt
> @@ -5,3 +5,4 @@ This tests for a crash when indenting a particular selection that contains a blo
>  
>  
>  
> +
> diff --git a/LayoutTests/editing/execCommand/insert-ordered-list-expected.txt b/LayoutTests/editing/execCommand/insert-ordered-list-expected.txt
> new file mode 100644
> index 0000000..7997d03
> --- /dev/null
> +++ b/LayoutTests/editing/execCommand/insert-ordered-list-expected.txt
> @@ -0,0 +1,3 @@
> +SUCCESS
> +text
> +
> diff --git a/LayoutTests/editing/execCommand/insert-ordered-list.html b/LayoutTests/editing/execCommand/insert-ordered-list.html
> new file mode 100644
> index 0000000..f2d2df0
> --- /dev/null
> +++ b/LayoutTests/editing/execCommand/insert-ordered-list.html
> @@ -0,0 +1,24 @@
> +<BODY>
> +<u>
> +  <img><br>
> +  <div>
> +  <ul>
> +    <li>
> +      <span><u>
> +        <div style="text-align:right; display: inline;">
> +          <span><u>text</u></span>
> +        </div>
> +      </u></span>
> +    </li>
> +  </ul>
> +  </div>
> +</u>
> +</BODY>
> +<SCRIPT>
> +  if (window.layoutTestController)
> +      layoutTestController.dumpAsText();
> +  document.designMode = "on";
> +  document.execCommand("selectall");
> +  document.execCommand("insertorderedlist");
> +  document.getElementsByTagName('u')[0].innerHTML = 'SUCCESS';
> +</SCRIPT>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 8d01b57..86086ab 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,20 @@
> +2010-01-14  Tony Chang  <tony at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=32131
> +        Editing crash when trying to insert an ordered list.
> +        While moving paragraphs that have been added to the new list,
> +        we lose the position of the placeholder <br> causing us to delete
> +        a bunch of nodes we don't mean to delete.
> +
> +        Test: editing/execCommand/insert-ordered-list.html
> +
> +        * editing/CompositeEditCommand.cpp:
> +        (WebCore::CompositeEditCommand::moveParagraphs):
> +        * editing/DeleteSelectionCommand.cpp:
> +        (WebCore::DeleteSelectionCommand::calculateTypingStyleAfterDelete):
> +
>  2010-01-13  Gavin Barraclough  <barraclough at apple.com>
>  
>          Reviewed by Oliver Hunt.
> diff --git a/WebCore/editing/CompositeEditCommand.cpp b/WebCore/editing/CompositeEditCommand.cpp
> index e9b6971..c69e84a 100644
> --- a/WebCore/editing/CompositeEditCommand.cpp
> +++ b/WebCore/editing/CompositeEditCommand.cpp
> @@ -946,10 +946,10 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
>      
>      setEndingSelection(VisibleSelection(start, end, DOWNSTREAM));
>      deleteSelection(false, false, false, false);
> -
>      ASSERT(destination.deepEquivalent().node()->inDocument());
>  
>      cleanupAfterDeletion();
> +    ASSERT(destination.deepEquivalent().node()->inDocument());
>  

I dont' understand why you moved the ASSERT here. In moveparagraph destination
should always be valid after deleteSelection, because destination represents
the
new location to move to and deleteSelection only affects the old stuff.

>      // Add a br if pruning an empty block level element caused a collapse. For example:
>      // foo^
> @@ -971,6 +971,7 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
>      destinationIndex = TextIterator::rangeLength(startToDestinationRange.get(), true);
>      
>      setEndingSelection(destination);
> +    ASSERT(endingSelection().isCaretOrRange());
>      applyCommandToComposite(ReplaceSelectionCommand::create(document(), fragment, true, false, !preserveStyle, false, true));
>      
>      // If the selection is in an empty paragraph, restore styles from the old empty paragraph to the new empty paragraph.
> diff --git a/WebCore/editing/DeleteSelectionCommand.cpp b/WebCore/editing/DeleteSelectionCommand.cpp
> index f07f038..3dd1490 100644
> --- a/WebCore/editing/DeleteSelectionCommand.cpp
> +++ b/WebCore/editing/DeleteSelectionCommand.cpp
> @@ -688,6 +688,8 @@ void DeleteSelectionCommand::calculateTypingStyleAfterDelete()
>          applyStyle(m_typingStyle.get(), EditActionUnspecified);
>          // applyStyle can destroy the placeholder that was at m_endingPosition if it needs to 
>          // move it, but it will set an endingSelection() at [movedPlaceholder, 0] if it does so.
> +        // We don't want to move endingSelection(), because another command might be relying on it.
> +        setEndingSelection(visibleEnd);

I believe the 19087.html is failing because you are moving the end selection in
a case where it is not necessary.
You should try to find out why this test fails, I don't think you should just
re-baseline the test without an explanation.

>          m_endingPosition = endingSelection().start();
>          m_typingStyle = 0;
>      }

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



More information about the webkit-unassigned mailing list