Bug 43834 - merge MarkupAccumulator and MarkupAccumulatorWrapper
Summary: merge MarkupAccumulator and MarkupAccumulatorWrapper
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: Ryosuke Niwa
URL:
Keywords:
Depends on: 43227
Blocks: 43936
  Show dependency tree / graph
 
Reported: 2010-08-11 00:42 PDT by Ryosuke Niwa
Modified: 2010-08-12 15:32 PDT (History)
6 users (show)

See Also:

Attachments
first attempt to merge (12.43 KB, patch)
2010-08-11 00:48 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch (10.25 KB, patch)
2010-08-11 12:28 PDT, Ryosuke Niwa 		tkent: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 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.
Comment 2 Ryosuke Niwa 2010-08-11 12:28:18 PDT 
Created attachment 64148 [details]
Patch
Comment 3 Ryosuke Niwa 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.
Comment 4 Kent Tamura 2010-08-12 01:53:47 PDT 
Comment on attachment 64148 [details]
Patch

Looks good.
Comment 5 Ryosuke Niwa 2010-08-12 14:06:55 PDT 
Committed r65265: <http://trac.webkit.org/changeset/65265>