[Webkit-unassigned] [Bug 48886] Avoid CFAttributedString creation in ComplexTextController

[bugzilla-daemon at webkit.org]

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


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #72781|review?                     |review-
               Flag|                            |




--- Comment #2 from mitz at webkit.org  2010-11-02 21:15:24 PST ---
(From update of attachment 72781)
View in context: https://bugs.webkit.org/attachment.cgi?id=72781&action=review

r- because this will break the Snow Leopard build. Also, a couple of minor comments.

> WebCore/WebCore.exp.in:1161
> +_wkCreateCTTypesetterWithUniCharProviderAndOptions

This will break the Leopard and Snow Leopard builds, because this symbols is not defined on those platforms. You should make a new #if !defined(BUILDING_ON_LEOPARD) && !defined(BUILDING_ON_SNOW_LEOPARD) section further down and put this export in that section. Then the export file generator will omit it from the export list for those platforms.

> WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:119
> +    return &info->cp[stringIndex];

I would write this as info->cp + stringIndex but either way is fine.

> WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:145
> +        ProviderInfo info = {cp, length, fontData->getCFStringAttributes(m_font.typesettingFeatures())};

WebKit style is to put spaces inside the braces (see the initialization of rtlOptionValues[] above). Another thing you could do is define a constructor for ProviderInfo.

> WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp:155
> +        ProviderInfo info = {cp, length, fontData->getCFStringAttributes(m_font.typesettingFeatures())};

Ditto

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