Bug 130811 - Convert more of HTMLConverter to C++
Summary: Convert more of HTMLConverter to C++
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-26 18:41 PDT by Sam Weinig
Modified: 2014-03-26 20:23 PDT (History)
0 users

See Also:


Attachments
Patch (34.22 KB, patch)
2014-03-26 18:42 PDT, Sam Weinig
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>