[Webkit-unassigned] [Bug 44288] MarkupAccumulator::appendStartMarkup should be broken down into pieces

[bugzilla-daemon at webkit.org]

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


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64993|review?                     |review+
               Flag|                            |




--- Comment #11 from Eric Seidel <eric at webkit.org>  2010-08-23 17:43:08 PST ---
(From update of attachment 64993)
WebCore/editing/markup.cpp:490
 +      const QualifiedName* parentName = 0;
I'm not sure I understand why we use a pointer here.  It seems using a QualifiedName local would work just as well. I can't imaging its any slower since QualifiedName it itself just a RefPtr.

WebCore/editing/markup.cpp:489
 +      bool documentIsHTML = text->document()->isHTMLDocument();
Seems this is only used in one place in this function.

WebCore/editing/markup.cpp:580
 +              append(out, attribute->name().localName());
is localName all caps or something?  Confused why this is needed.

WebCore/editing/markup.cpp:587
 +              // We don't want to complete file:/// URLs because it may contain sensitive information
Are there other file-like urls that could get us into trouble?

WebCore/editing/markup.cpp:604
 +          RefPtr<CSSMutableStyleDeclaration> style = static_cast<HTMLElement*>(element)->getInlineStyleDecl()->copy();
Wow, this block wants to be broken up again too. :)

r+ since you're just moving code.  But you might consider my comments for a follow-up patch.  I suspect we also need more testing here, but that can come when we start fixing stuff here. :)

Thank you SO MUCH for working on this!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.