[Webkit-unassigned] [Bug 131839] The OS/2 USE_TYPO_METRICS fsSelection flag is not taken into account

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #263907|review?                     |review+
              Flags|                            |

--- Comment #11 from Darin Adler <darin at apple.com> ---
Comment on attachment 263907
  --> https://bugs.webkit.org/attachment.cgi?id=263907
Patch iOS 2

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

I’m saying review+, but please do *not* land this patch exactly as is.

> Source/WebCore/platform/graphics/ios/FontServicesIOS.mm:38
> +static bool fontHasMathTable(CTFontRef ctFont)

This function already exists in FontCocoa.mm. Please don’t write a second copy of it; instead lets find a way to share the existing function.

> Source/WebCore/platform/graphics/ios/FontServicesIOS.mm:125
> +                if (*(reinterpret_cast<const OpenType::UInt16*>(os2Data + fsSelectionOffset)) & useTypoMetricsMask) {

I suggest writing a helper function to do this messy reinterpret_cast thing that is repeated four times.

> Source/WebCore/platform/graphics/opentype/OpenTypeTypes.h:32
> +#if ENABLE(OPENTYPE_MATH)
> +#include "Glyph.h"
> +#endif

I don’t understand why this include needs to be added to a header, when we aren’t touching the header otherwise. If this is needed in FontServicesIOS.mm, then please put it in there.

Also, the header isn’t mentioned in the change log.

Please don’t land this header change!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151031/99dbc655/attachment-0001.html>