Bug 101799 – SimpleFontData::getCFStringAttributes sets some attributes to their default values, but shouldn’t

• Component: Text
• Status: RESOLVED FIXED

<rdar://problem/12290266>

In some configurations, setting kCTLigatureAttributeName to its default value of 1 yields different behavior from not setting it at all. We can get the correct behavior and greatly simplify the code by not setting attributes to their default values.

Created attachment 173370 [details]
Only include attributes with non-default values

[Alexey Proskuryakov]

Comment on attachment 173370 [details]
Only include attributes with non-default values

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

> Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:42
> +    attributesDictionary.adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 4, &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

This is a fairly magical "4".

> Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:55
> +        static int zero = 0;

Why not a "const"?

Thanks for the review!

(In reply to comment #2)
> (From update of attachment 173370 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173370&action=review
> 
> > Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:42
> > +    attributesDictionary.adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 4, &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
> 
> This is a fairly magical "4".

How do you propose to resolve this? Named constant? Descriptive comment? Other?

> > Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:55
> > +        static int zero = 0;
> 
> Why not a "const"?

No reason.

[Alexey Proskuryakov]

> How do you propose to resolve this? Named constant? Descriptive comment? Other?

No specific proposal.

Fixed in <http://trac.webkit.org/r134146>.

[WebKit Review Bot]

Re-opened since this is blocked by bug 101851

(In reply to comment #6)
> Re-opened since this is blocked by bug 101851

This makes no sense.