[Webkit-unassigned] [Bug 26949] WebCore part of the Haiku WebKit port

[bugzilla-daemon at webkit.org]

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


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32924|review?                     |review-
               Flag|                            |




--- Comment #38 from Eric Seidel <eric at webkit.org>  2009-08-07 11:05:03 PDT ---
(From update of attachment 32924)
What is this and why?
 // Fix temp routine to check the conversion works, please move it ASAP    
 44 int charUnicodeToUtf8(unsigned short glyph, char* out)

It's not mentioned in the ChangeLog.  I needs a more decriptive name if it's
just a hack.

convertUTF16ToUTF8RenderingHACK or something.

We don't use argument names in headers when they don't add value:
47         FontPlatformData(const FontDescription& fontDescription, const
AtomicString& family);
 48         FontPlatformData(const FontDescription& fontDescription, BFont*
font);
 49         FontPlatformData(float size, bool bold, bool italic);
 50         FontPlatformData(const FontPlatformData& fontPlatformData);
"family" can stay.  but the fontDescription at least shoudl go.

No hungarian notation here ;)
 66     bool bFound = false;
 67     bool bItalic = fontDescription.italic();
 68     bool bBold = fontDescription.weight() == FontWeightBold;
We ain't no microsoft...

Why isn't this an OwnPtr<BFont>:
 63         BFont* m_font;

Your leaking BFonts.

Why is this correct?
        if (isUtf16) {
 50             UChar lead = characterBuffer[i * 2];
 51             UChar trail = characterBuffer[i * 2 + 1];
 52             character = U16_GET_SUPPLEMENTARY(lead, trail);
 53         } else

Not all characters will be composed characters.

Use the height of the X, can't you ask the font for the height of a glyph at a
certain size?
 52         m_xHeight = font->Size(); // Not sure about this

This information should be on the BFont:
 53         m_unitsPerEm = 1; // FIXME!

You'd have more luck posting this in smaller pices.  Some of this was just
fine, but some of the font code is wrong here too.  If you broke it out, I
coudl more easily approve the OK parts.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.