[Webkit-unassigned] [Bug 53564] Make canHaveChildrenForEditing more efficient

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Ryosuke Niwa <rniwa at webkit.org>  2011-02-02 16:34:38 PST ---
(In reply to comment #6)
> (From update of attachment 80996 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80996&action=review
> 
> It kinda sucks to polute Node.h like this.  I mean, certainly we follow this pattern for other areas of the code, but I wonder if in this case your original hashset isn't better?

I think it's okay to add a member function to Node in this case because the concept of not being able to have children is also related to Position's behavior.  For example, positions or range boundary points like [br, 0] or [img, 0] doesn't make sense, and this is not necessarily editing-specific knowledge.

Maybe we should rename it to something more general like canHaveChildrenForPosition.   I was also thinking that we can have canHaveOffsetInside to replace editingIgnoresContent, and have canHaveChildrenForEditing in htmlediting.h instead because there are very few places where we call canHaveChildrenForPosition but there are many places where we call editingIgnoresContent.

>Are there more general rules about what types of tags can/can't have editing kids?  should we move the false up to some PluginElement superclass instead of <object>, <embed> directly?

I do return false in HTMLPlugInElement.h.  I listed all elements in ChangeLog just for the documentation purpose.

> > Source/WebCore/dom/Node.h:234
> > +    // FIXME: We should return false here and return true in ContainerNode
> > +    virtual bool canHaveChildrenForEditing() const { return true; }
> 
> Why not just fix this?

It seemed a risky change at the moment but I could do that.

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