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

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Enrica Casucci <enrica at apple.com>  2010-01-18 11:04:27 PST ---
(In reply to comment #7)
> (In reply to comment #6)
> > > --- 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.
> 
> I was removing a blank line and adding another ASSERT.  The destination needs
> to be valid after the cleanup so we know where to insert paragraph into.
> 
> 
The point I'm trying to make is that it is not necessary to move the ASSERT.
Destination is a position that outside the selection being deleted. The ASSERT
should verify that there is no overlap between destination and the selection to
be deleted. We should not make unnecessary code changes.

> > 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.
> 
> You're right, the bug is lower: it's in applyStyle.  In
> ApplyStyleCommand::applyBlockStyle, it tries to save the selection, move the
> paragraphs, then restore the selection.  In this test case, it fails to restore
> the selection to the right place.  It looks like this might be a bug in
> TextIterator::rangeLength and TextIterator::rangeFromLocationAndLength not
> agreeing on how to restore a range?  I'm not sure what rangeLength is supposed
> to represent.
I don't agree. The test is failing because one line is left behind and I
believe it has to do with moving the end selection. TextIterator computes
correctly the position starting from an offset expressed in terms of number of
visible characters. It could be off only if a layout is needed.

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