Bug 130811

Summary: Convert more of HTMLConverter to C++
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch andersca: review+

Description Sam Weinig 2014-03-26 18:41:00 PDT
Convert more of HTMLConverter to C++
Comment 1 Sam Weinig 2014-03-26 18:42:03 PDT
Created attachment 227907 [details]
Patch
Comment 2 Anders Carlsson 2014-03-26 19:11:50 PDT
Comment on attachment 227907 [details]
Patch

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

> Source/WebCore/editing/cocoa/HTMLConverter.mm:1357
> +    auto result = m_attributesForElements.add(&coreElement, nullptr);
> +    if (result.isNewEntry)
> +        result.iterator->value = _computedAttributesForElement(coreElement);

I think you should do 

auto& attributes = m_attributesForElements.add(&coreElement, nullptr).iterator->value;
if (!attributes)
    attributes = _computedAttributesForElement(coreElement);

> Source/WebCore/editing/cocoa/HTMLConverter.mm:1566
> +    NSString *width = element.getAttribute(WebCore::HTMLNames::widthAttr);

Does this do the right thing for table cell elements now?
Comment 3 Sam Weinig 2014-03-26 19:19:41 PDT
(In reply to comment #2)
> (From update of attachment 227907 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227907&action=review
> 
> > Source/WebCore/editing/cocoa/HTMLConverter.mm:1357
> > +    auto result = m_attributesForElements.add(&coreElement, nullptr);
> > +    if (result.isNewEntry)
> > +        result.iterator->value = _computedAttributesForElement(coreElement);
> 
> I think you should do 
> 
> auto& attributes = m_attributesForElements.add(&coreElement, nullptr).iterator->value;
> if (!attributes)
>     attributes = _computedAttributesForElement(coreElement);

Will do! Though I really want to add my m_attributesForElements.add(key, [] { return computeNewValue(); }); patch one day when we can think of a good name for add() :(.

> 
> > Source/WebCore/editing/cocoa/HTMLConverter.mm:1566
> > +    NSString *width = element.getAttribute(WebCore::HTMLNames::widthAttr);
> 
> Does this do the right thing for table cell elements now?

Yeah, [(DOMHTMLTableCellElement *)element width] just called getAttribute(WebCore::HTMLNames::widthAttr);
Comment 4 Sam Weinig 2014-03-26 20:23:22 PDT
Committed r166335: <http://trac.webkit.org/changeset/166335>