[webkit-reviews] review denied: [Bug 28131] [Haiku] Adding font-specific files to WebCore. : [Attachment 34422] Patch to add four other font-specific files to WebCore/platform/graphics/haiku/.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Aug 9 08:53:28 PDT 2009

Eric Seidel <eric at webkit.org> has denied Maxime Simon
<simon.maxime at gmail.com>'s request for review:
Bug 28131: [Haiku] Adding font-specific files to WebCore.

Attachment 34422: Patch to add four other font-specific files to

------- Additional Comments from Eric Seidel <eric at webkit.org>
naming style:
 71	static AtomicString s_DefaultStr(family);

This should indicate that it's a hack somehow:
1 // Fix temp routine to check the conversion works, please move it ASAP
 42 int charUnicodeToUtf8(unsigned short glyph, char* out)
it's not clear to me from reading that method name that this method isn't a
good thing. :)	Although since I started at it enought o see what it does,
you're basically just flattening all passed in chars to utf8 so that you can
pass them off to B drawing routines.  This is a hack, and needs to be
designated as such.

Shouldn't char* out be char[4]& out?

Don't we already have typedefs making these casts uneeded?
2     BView* view = static_cast<BView*>(graphicsContext->platformContext());
 73	BFont* m_font = static_cast<BFont*>(font->platformData().font());

Why is the const_cast needed?
 79	GlyphBufferGlyph* glyphs =

Early return is preferred:
45     BFont* font = m_platformData.font();
 46	if (font) {
if (!font)

It seems the handling of m_smallCapsFontData is wrong on other ports.  But I'm
not convinced your handling is correct either.	It certainly is strange that
SimpleFontData.cpp would try to delete it too.

     if (m_platformData.font())
 90	    m_treatAsFixedPitch = m_platformData.font()->IsFixed();
 91	else
 92	    m_treatAsFixedPitch = false;

m_treadAsFixedPitch = m_platformData.font() &&

What is an "escapement"?  Wow a B-only term it seems:

More information about the webkit-reviews mailing list