[Webkit-unassigned] [Bug 20092] Spelling markers positioned incorrectly in RTL text

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

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




------- Comment #5 from darin at apple.com  2008-09-22 16:52 PDT -------
(From update of attachment 23682)
A patch needs to include a test case and a ChangeLog entry. This doesn't have
either. Dan Bernstein can help you make a "pixel test" for this, or you could
construct a manual test case for the manual-tests directory.

 659     int startPosition = max(marker.startOffset - m_start, (unsigned)0);

This is wrong. Since both expressions are of type "unsigned", then it's going
to be an unsigned "max", so it will always be startOffset - start, even if it's
a negative number. Instead of casting the right side to unsigned we need to
cast the left side to int.

I'd like to see test cases that exercise this "max" statement. Or we can leave
the max out if it's not really needed -- if we can't create a test case that is
affected then maybe it's not.

 660     int endPosition = min(marker.endOffset - m_start, (unsigned)m_len);

 677         int grammarEndPosition = min(marker.endOffset - m_start,
(unsigned)m_len);

Might be the same problem here. We don't want a negative number to turn into
m_len, so we need to do an int version of min instead of unsigned.

It occurs to me that another way to accomplish this is to call min<int>(a, b)
instead of casting a or b to an unsigned.

 662     if (m_truncation != cNoTruncation) {
 663         endPosition = min(endPosition, startPosition + m_truncation);
 664     }

Out style guide doesn't use braces around the bodies of single line if
statements.

review- because of the issues above


-- 
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.