Bug 45449

Summary: Extract the code to find special ancestors in createMarkup into a function
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, darin, enrica, eric, ojan, tkent, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 34564    
Attachments:
Description Flags
Patch tony: review+

Ryosuke Niwa
Reported 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.
Attachments
Patch (12.72 KB, patch)
2010-09-09 03:09 PDT, Ryosuke Niwa
tony: review+
Ryosuke Niwa
Comment 1 2010-09-09 03:09:00 PDT
Tony Chang
Comment 2 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.
Ryosuke Niwa
Comment 3 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.
Ryosuke Niwa
Comment 4 2010-09-10 00:13:47 PDT
WebKit Review Bot
Comment 5 2010-09-10 00:32:08 PDT
http://trac.webkit.org/changeset/67176 might have broken Qt Linux Release
Note You need to log in before you can comment on or make changes to this bug.