Bug 167062 – De-duplicate some (nearly) identical code in Editor(Mac|IOS).mm

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

De-duplicate some (nearly) identical code in Editor(Mac|IOS).mm

[Tim Horton]

Created attachment 298876 [details]
Patch

Comment on attachment 298876 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        Move these six methods here.

functions

> Source/WebCore/ChangeLog:29
> +        and Editor(Mac|IOS), so we'll make them class methods.

static member functions

> Source/WebCore/editing/Editor.cpp:3657
> +        const Font* result = nullptr;

Should we call this font like the other one below?

> Source/WebCore/editing/Editor.cpp:3670
> +    if (range && startNode) {

Maybe change this to an early return and only declare the font variable after?

> Source/WebCore/editing/Editor.cpp:3672
> +        // In the loop below, n should eventually match pastEnd and not become nil, but we've seen at least one

null

> Source/WebCore/editing/Editor.h:446
> +    WEBCORE_EXPORT const Font* fontForSelection(bool&) const;

The boolean needs to be named here.

> Source/WebCore/editing/cocoa/EditorCocoa.mm:133
> +    content.dataInRTFDFormat = attributedString.containsAttachments ? dataInRTFDFormat(attributedString) : 0;

nullptr?

[Tim Horton]

Created attachment 298880 [details]
Patch

[Darin Adler]

Comment on attachment 298876 [details]
Patch

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

>> Source/WebCore/editing/Editor.h:446
>> +    WEBCORE_EXPORT const Font* fontForSelection(bool&) const;
> 
> The boolean needs to be named here.

Another option that could clean things up in the future would to change this to return a struct so we can have both the font and the "has multiple fonts" boolean without using an out argument. Even a tuple or pair might be better, but then we would not have a name for the boolean.

[Tim Horton]

Created attachment 298883 [details]
Patch

[Tim Horton]

Created attachment 298887 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 298887 [details]
Patch

Clearing flags on attachment: 298887

Committed r210773: <http://trac.webkit.org/changeset/210773>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.