Bug 45449 - Extract the code to find special ancestors in createMarkup into a function
Summary: Extract the code to find special ancestors in createMarkup into a function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 34564
  Show dependency treegraph
 
Reported: 2010-09-09 03:01 PDT by Ryosuke Niwa
Modified: 2010-09-10 00:32 PDT (History)
8 users (show)

See Also:


Attachments
Patch (12.72 KB, patch)
2010-09-09 03:09 PDT, Ryosuke Niwa
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2010-09-09 03:01:09 PDT
In order to fix the bug 34564, we must isolate the code to find the special common ancestor into a function.
Comment 1 Ryosuke Niwa 2010-09-09 03:09:00 PDT
Created attachment 67014 [details]
Patch
Comment 2 Tony Chang 2010-09-09 11:27:41 PDT
Comment on attachment 67014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67014&action=prettypatch

> WebCore/editing/markup.cpp:961
> +    Node* specialCommonAncestor = 0;
> +    if (shouldAnnotate == AnnotateForInterchange) {
> +        specialCommonAncestor = ancestorToRetainStructureAndAppearance(commonAncestor);
Nit: You may want to copy the comment that was around here about which ancestor you want to include.  It's not obvious why we might want the specialCommonAncestor (ie., what makes it special).  Maybe the comment should just go above this function.
Comment 3 Ryosuke Niwa 2010-09-10 00:07:43 PDT
Thanks for the review.  Sorry about a delayed commit.  I had to push regression fix first.

(In reply to comment #2)
> (From update of attachment 67014 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67014&action=prettypatch
> 
> > WebCore/editing/markup.cpp:961
> > +    Node* specialCommonAncestor = 0;
> > +    if (shouldAnnotate == AnnotateForInterchange) {
> > +        specialCommonAncestor = ancestorToRetainStructureAndAppearance(commonAncestor);
> Nit: You may want to copy the comment that was around here about which ancestor you want to include.  It's not obvious why we might want the specialCommonAncestor (ie., what makes it special).  Maybe the comment should just go above this function.

Will do.
Comment 4 Ryosuke Niwa 2010-09-10 00:13:47 PDT
Committed r67176: <http://trac.webkit.org/changeset/67176>
Comment 5 WebKit Review Bot 2010-09-10 00:32:08 PDT
http://trac.webkit.org/changeset/67176 might have broken Qt Linux Release