Bug 32079

Summary: Undo should not affect elements that are not editable
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: HTML EditingAssignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
patch adele: review+

Description Enrica Casucci 2009-12-02 11:21:46 PST
If an element changes from editable to non-editable, all undo commands that apply to it and its descendant should be ignored.
Same goes for redo.
Comment 1 Enrica Casucci 2009-12-02 11:56:37 PST
Created attachment 44170 [details]
patch
Comment 2 WebKit Review Bot 2009-12-02 21:10:58 PST
style-queue ran check-webkit-style on attachment 44170 [details] without any errors.
Comment 3 Adele Peterson 2009-12-02 22:52:13 PST
Comment on attachment 44170 [details]
patch

Are there cases where the parent isn't editable, and its not attached, where we would want to allow the AppendNodeCommand?

>  void AppendNodeCommand::doApply()
>  {
> +    if (!m_parent->isContentEditable() && m_parent->attached())
> +        return;
> +        
>      ExceptionCode ec;
>      m_parent->appendChild(m_node.get(), ec);
>  }
Comment 4 Enrica Casucci 2009-12-03 10:10:40 PST
(In reply to comment #3)
> (From update of attachment 44170 [details])
> Are there cases where the parent isn't editable, and its not attached, where we
> would want to allow the AppendNodeCommand?
> 
> >  void AppendNodeCommand::doApply()
> >  {
> > +    if (!m_parent->isContentEditable() && m_parent->attached())
> > +        return;
> > +        
> >      ExceptionCode ec;
> >      m_parent->appendChild(m_node.get(), ec);
> >  }

I don't think so. Look at the ASSERT in the constructor.
    ASSERT(m_parent->isContentEditable() || !m_parent->attached());
Comment 5 Adele Peterson 2009-12-03 10:42:05 PST
ok, makes sense
Comment 6 Enrica Casucci 2009-12-03 11:22:27 PST
Committed revision 51645.