Bug 167871 – Read stroke width of subtitles from MediaAccessibility framework.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Per Arne Vollan]

We can receive MACaptionFontAttributeStrokeWidth embedded in the CTFontDescriptor.

[Per Arne Vollan]

Created attachment 300699 [details]
Patch

[Per Arne Vollan]

Created attachment 300705 [details]
Patch

[Brent Fulgham]

Comment on attachment 300705 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=167871

Can you include the relevant Radar here, please?

> Source/WebCore/ChangeLog:12
> +        (WebCore::CaptionUserPreferencesMediaAF::captionsTextEdgeCSS):

You can delete this line. "prepare-ChangeLog" misunderstood the whitespace change and though 'captionsTextEdgeCSS' was updated, too.

> Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:393
> +    if (!MACaptionFontAttributeStrokeWidth)

We should build a test system for this, possibly as a TestWebKitAPI test where you set the MACaptionFontAttributeStrokeWidth and confirm that the resulting rendering is correct.

I'm fine with you doing this as a separate patch dealing with macOS and Windows.

> Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:396
> +    RetainPtr<CTFontDescriptorRef> font = adoptCF(MACaptionAppearanceCopyFontDescriptorForStyle(kMACaptionAppearanceDomainUser, nullptr, kMACaptionAppearanceFontStyleDefault));

Could this be auto?

> Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:400
> +    RetainPtr<CFTypeRef> strokeWidthAttribute = adoptCF(CTFontDescriptorCopyAttribute(font.get(), MACaptionFontAttributeStrokeWidth));

Ditto auto?

[Per Arne Vollan]

(In reply to comment #3)
> Comment on attachment 300705 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300705&action=review
> 
> > Source/WebCore/ChangeLog:4
> > +        https://bugs.webkit.org/show_bug.cgi?id=167871
> 
> Can you include the relevant Radar here, please?
> 
> > Source/WebCore/ChangeLog:12
> > +        (WebCore::CaptionUserPreferencesMediaAF::captionsTextEdgeCSS):
> 
> You can delete this line. "prepare-ChangeLog" misunderstood the whitespace
> change and though 'captionsTextEdgeCSS' was updated, too.
> 
> > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:393
> > +    if (!MACaptionFontAttributeStrokeWidth)
> 
> We should build a test system for this, possibly as a TestWebKitAPI test
> where you set the MACaptionFontAttributeStrokeWidth and confirm that the
> resulting rendering is correct.
> 
> I'm fine with you doing this as a separate patch dealing with macOS and
> Windows.
> 
> > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:396
> > +    RetainPtr<CTFontDescriptorRef> font = adoptCF(MACaptionAppearanceCopyFontDescriptorForStyle(kMACaptionAppearanceDomainUser, nullptr, kMACaptionAppearanceFontStyleDefault));
> 
> Could this be auto?
> 
> > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:400
> > +    RetainPtr<CFTypeRef> strokeWidthAttribute = adoptCF(CTFontDescriptorCopyAttribute(font.get(), MACaptionFontAttributeStrokeWidth));
> 
> Ditto auto?

Thanks for reviewing :) I will update the patch.

[Per Arne Vollan]

Created attachment 300799 [details]
Patch

[Per Arne Vollan]

<rdar://problem/29265906>

[Per Arne Vollan]

Committed <https://trac.webkit.org/changeset/211803>