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

[bugzilla-daemon at webkit.org]

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.
https://bugs.webkit.org/show_bug.cgi?id=28131

Attachment 34561: Patch to add four other font-specific files to
WebCore/platform/graphics/haiku/.
https://bugs.webkit.org/attachment.cgi?id=34561&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Why execute the family code ever time when you're only creating the
AtomicString once?
 69	font_family family;
 70	defaultFont->GetFamilyAndStyle(&family, 0);
 71	static AtomicString defaultString(family);

Seems like a bad idea...
 bool FontData::containsCharacters(const UChar* characters, int length) const
 43 {
 44	notImplemented();
 45	return true;
 46 }

Certainly will make international sites look awful. :)

This is not an english sentence:
 41 // FIXME: Temp routine to check the conversion works.
I'm not quite sure what you mean by that.

Is it possible to pass these through to the simple versions?
 97 float Font::floatWidthForComplexText(const TextRun& run, HashSet<const
SimpleFontData*>* fallbackFonts) const

I'm not sure that it would help you any, but it might.

I take it Haiku does not have easy options for drawing complex text?

Not a good variable name:
 76	    const FontPlatformData* pdata = new FontPlatformData(desc,
desc.family().family());
 74	    FontDescription desc = FontDescription(fontDescription);

Otherwise looks fine.