[Webkit-unassigned] [Bug 67056] Logic from HTMLElement::deprecatedCreateContextualFragment moved into Range::createContextualFragment function

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Sep 4 11:58:28 PDT 2011


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





--- Comment #26 from Kaustubh Atrawalkar <kaustubh at motorola.com>  2011-09-04 11:58:28 PST ---

> I don't think this function name is appropriate because element and its children are already in the fragment at this point. What this function is doing is to remove element preserving children.  I'd say removeElementPreservingChildren will be a better name (I'm kind of sad about the fact we can't share code with RemoveNodePreservingChildrenCommand).
> 
Hmm..seems to be relevant name. Ok I changed it.
> I don't see any benefit in declaring in this variable.  You should just do:
> for (RefPtr<Node> child = element->firstChild(); child; child = nextChild)
> 
Just habit of getting pointers into variable before using it. Need to break it :)
> This should be a single else-if
> 
Actually it was copy pasted from previous code. So didn't changed it much. But will make it.
> This comment should either be on the same line as return 0; or be wrapped by curly brackets (because the comment & return 0 occupy 2 lines).
> 
Yep. Done this.

> This function can be static, no?

Yes this can be static. And then subsequent usage can also be without creating object of Range. So much more beneficial declaring it static. I missed that during splitting of the function createContextualFragment. Thanks you got that.

-- 
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