[Webkit-unassigned] [Bug 18413] hixie test fails for CSS: sanity tests for relative keyword values of font-size
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jan 6 11:48:12 PST 2009
https://bugs.webkit.org/show_bug.cgi?id=18413
mitz at webkit.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #26360|review? |review-
Flag| |
------- Comment #9 from mitz at webkit.org 2009-01-06 11:48 PDT -------
(From update of attachment 26360)
Thanks for coming back to this.
> + int i;
In WebKit C++ code, loop iterators are usually defined inside the for
statement. If the iterator is used outside the loop's scope, then it should be
defined just before the loop.
> + const int *currentFontSizeTable;
The * should go next to the type.
> + currentFontSizeTable = const_cast<int *> (customFontSizeTable);
There should not be a space before the *. There also shouldn't be a space
before the left parenthesis.
> + // If outside the scope of the table, return approximate value.
> + if (size >= currentFontSizeTable[totalKeywords - 1])
> + return increaseFontSize ? size * (static_cast<float> (currentFontSizeTable[totalKeywords - 1]) / static_cast<float> (currentFontSizeTable[totalKeywords - 2]))
> + : size / (static_cast<float> (currentFontSizeTable[totalKeywords - 1]) / static_cast<float> (currentFontSizeTable[totalKeywords - 2]));
> +
> + if (size < currentFontSizeTable[0] || (size == currentFontSizeTable[0] && !increaseFontSize))
> + return size + static_cast<float> (modifier);
Why is adding or subtracting 1 the correct behavior for sizes smaller than the
smallest size in the table? Wouldn't a multiplicative scale factor make more
sense? The asymmetry with the other side is odd.
> + int currentMatch = 0;
> + float deviation = static_cast<float> (INT_MAX);
There should not be a space before the left parenthesis.
> + // find which two values from the table the size falls between
> + for (i = 0; i < totalKeywords - 1; i++) {
> + if (size - currentFontSizeTable[i] < currentFontSizeTable[i + 1] - currentFontSizeTable[i] && size >= currentFontSizeTable[i]) {
No need to subtract currentFontSizeTable[i] from both sides of the inequality.
> + float newDeviation;
> + float position = deviation / static_cast<float> (currentFontSizeTable[currentMatch + 1] - currentFontSizeTable[currentMatch]);
No need to cast because deviation is a float.
> + if (increaseFontSize) {
> + if (currentMatch == totalKeywords - 2)
> + newDeviation = position * ((static_cast<float> (currentFontSizeTable[currentMatch+1]) * static_cast<float> (currentFontSizeTable[currentMatch+1]) / static_cast<float> (currentFontSizeTable[currentMatch]))
> + - static_cast<float> (currentFontSizeTable[currentMatch + 1]));
Not all of the casts are necessary. There should be spaces around the +
operator and no spaces between > and (.
> + else
> + newDeviation = position * static_cast<float> (currentFontSizeTable[currentMatch + 2] - currentFontSizeTable[currentMatch + 1]);
> + } else
> + newDeviation = position * static_cast<float> (currentFontSizeTable[currentMatch] - currentFontSizeTable[currentMatch - 1]);
How do you know that currentMatch is greater than 0?
> + if (roundf(newDeviation) == 0) {
> + fontDescription.setKeywordSize(currentMatch + modifier + 1);
> + return currentFontSizeTable[currentMatch + modifier];
> + }
> +
> + return currentFontSizeTable[currentMatch + modifier] + newDeviation;
Same question.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list