[webkit-reviews] review granted: [Bug 15312] WebKit applies an incorrect min width for <input type="text"> fields : [Attachment 29764] WebCore side of patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 28 21:33:10 PDT 2009
mitz at webkit.org has granted Ojan Vafai <ojan at chromium.org>'s request for
review:
Bug 15312: WebKit applies an incorrect min width for <input type="text"> fields
https://bugs.webkit.org/show_bug.cgi?id=15312
Attachment 29764: WebCore side of patch.
https://bugs.webkit.org/attachment.cgi?id=29764&action=review
------- Additional Comments from mitz at webkit.org
> + if (glyphPageZero) {
> + static int weights[] = { 64, 14, 27, 35, 100, 20, 14, 42, 63, 3, 6,
35, 20, 56, 56, 17, 4, 49, 56, 71, 31, 10, 18, 3, 18, 2, 166 };
> + float sum = 0.f;
> + int numGlyphs = 27;
> + ASSERT(numGlyphs == sizeof(weights)/sizeof(int));
Please add spaces around the / sign.
> + // Compute the weighted sum of the space character and the lowercase
letters in the Latin alphabet.
> + for (int i = 0; i < numGlyphs; i++) {
> + Glyph glyph = glyphPageZero->glyphDataForCharacter((i < 26 ? i +
'a' : ' ')).glyph;
> + if (glyph)
> + sum += widthForGlyph(glyph) * weights[i];
> + }
> + m_avgCharWidth = sum / 1000;
It may be better to add up the total weight of found glyphs and divide by that
instead of 1000. Consider a font that contains a space glyph but no Latin
(Geeza Pro is an example, if I remember correctly).
r=me , but I suggest making the above change before landing the patch.
More information about the webkit-reviews
mailing list