[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


Oliver Hunt <oliver at apple.com> changed:

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




--- Comment #20 from Oliver Hunt <oliver at apple.com>  2009-07-16 01:03:54 PDT ---
(From update of attachment 32371)

> +// Fix temp routine to check the conversion works, please move it ASAP	
> +int charUnicodeToUtf8(unsigned short glyph, char* out)
> +{
> +    int i = 0;
> +
> +    if (glyph < 0x0080)
> +        out[i++] = (char)glyph;
C++ style casts please :D

> +    else if (glyph < 0x0800) { //two bytes
> +        out[i++] = 0xc0 | (( glyph ) >> 6 );
> +        out[i++] = 0x80 | (( glyph ) & 0x3F );
> +    } else if (glyph > 0x0800) { //3 bytes
> +        out[i++] = 0xe0 | (( glyph ) >> 12 );
> +        out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F );
> +        out[i++] = 0x80 | (( glyph ) & 0x3F);
> +    }
Should have excess braces, eg.
out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F ); => out[i++] = 0x80 | ((glyph >>
6 ) & 0x3F );
Shouldn't have spaces around the content of parenthesis, eg.
out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F ); => out[i++] = 0x80 | ((glyph >>
6) & 0x3F);


> +FontPlatformData::FontPlatformData(const FontDescription& fontDescription, const AtomicString& familyName)
> +    : m_font(0)
> +{
> +    bool bFound = false;
> +    bool bItalic = fontDescription.italic();
> +    bool bBold = fontDescription.weight() == FontWeightBold;
> +    font_family family;
> +
> +    for (int i = 0; i < count_font_families(); i++) {
> +        get_font_family(i, &family, 0);
> +        if (!BString(familyName.string()).Compare(family)) {
> +            m_font = new BFont();
> +            m_font->SetFamilyAndStyle(family, 0);
> +            m_font->SetSize(fontDescription.computedSize());
> +
> +            bFound = true;
> +            break;
> +        }
> +    }
> +
> +    if (bFound) {
> +        int32 numStyles = count_font_styles(family);
> +        font_style fontStyle;
> +        uint32 flags;
> +        bFound = false;
> +
> +        for (int i = 0; i < numStyles; i++) {
> +            if (get_font_style(family, i, &fontStyle, &flags) == B_OK) {
> +                String style(fontStyle);
> +                if ((bItalic && bBold)
> +                    && (style == "BoldItalic" || style == "Bold Oblique")) {
There's no reason for this line break, we don't have an 80-cloumn limit in our
style guidelines :D

> +                     bFound = true;
> +                     break;
> +                 }
> +                if ((bItalic && !bBold)
> +                    && (style == "Italic" || style == "Oblique")) {
ditto

...
> +                 }
> +                if ((!bItalic && !bBold)
> +                    && (style == "Roma"
> +                        || style == "Book" || style == "Condensed"
> +                        || style == "Regular" || style == "Medium")) {
You may want multiple lines here for clarity, but 4 lines seems excessive

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