Bug 43834

Summary: merge MarkupAccumulator and MarkupAccumulatorWrapper
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, darin, enrica, ojan, tkent, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 43227    
Bug Blocks: 43936    
Attachments:
Description Flags
first attempt to merge
none
Patch tkent: review+

Ryosuke Niwa
Reported 2010-08-11 00:42:25 PDT
We should merge MarkupAccumulator and MarkupAccumulatorWrapper. This is a follow up bug for https://bugs.webkit.org/show_bug.cgi?id=43227.
Attachments
first attempt to merge (12.43 KB, patch)
2010-08-11 00:48 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Patch (10.25 KB, patch)
2010-08-11 12:28 PDT, Ryosuke Niwa 		tkent: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2010-08-11 00:48:40 PDT
Created attachment 64084 [details] first attempt to merge This is my first attempt to merge the two classes. I merged MarkupAccumulator::appendMarkup and serializeNodes to centralize all serializations in one place. But I feel like this approach is wrong because serializations done in two different versions of createMarkup are so different. I should probably add a recursive serializeNodesWithNamespace to replace MarkupAccumulator::appendMarkup instead. serializeNodesWithNamespace can take MarkupAccumulatorWrapper as an argument and this will allow us to merge two classes.
Ryosuke Niwa
Comment 2 2010-08-11 12:28:18 PDT
Created attachment 64148 [details] Patch
Ryosuke Niwa
Comment 3 2010-08-11 12:32:53 PDT
(In reply to comment #2) > Created an attachment (id=64148) [details] > Patch In this patch, I didn't try to merge two serializations. But instead, I added serializeNodeWithNamespaces which uses MarkupAccumulatorWrapper for the node version of createMarkup. I'm intending to put most of functions in markup.cpp into MarkupAccumulatorWrapper and rename it to MarkupAccumulator once this patch is landed.
Kent Tamura
Comment 4 2010-08-12 01:53:47 PDT
Comment on attachment 64148 [details] Patch Looks good.
Ryosuke Niwa
Comment 5 2010-08-12 14:06:55 PDT
Committed r65265: <http://trac.webkit.org/changeset/65265>
Note You need to log in before you can comment on or make changes to this bug.