[Webkit-unassigned] [Bug 106815] [Chromium] Tests and fixes for spell checker behavior

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #185547|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #57 from Tony Chang <tony at chromium.org>  2013-01-30 14:52:49 PST ---
(From update of attachment 185547)
View in context: https://bugs.webkit.org/attachment.cgi?id=185547&action=review

Some minor style nits, but this looks OK overall.  Note that the Qt, Gtk and Efl bots don't run tests, so these tests may fail on those ports too.  You can either suppress the errors in TestExpectations or land them and watch the bots on build.webkit.org.

> Source/WebKit/chromium/src/ContextMenuClientImpl.cpp:296
> +                    // If there is no selection, then select the misspelling.

WebKit style is to avoid comments that only describe the code.  Most comments are for answering 'why' the code is there. I would remove this comment.

> Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:76
> +        int maxWordLength = stringText.length() - wordOffset;
> +        int wordLength;

Don't you need to static_cast length() to an int?  I think some compilers will complain about this signed/unsigned conversion.  Should maxWordLength and wordLength be unsigned?

> Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:77
> +        WTF::String word;

Nit: No WTF:: needed.

> Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:85
> +            wordLength = m_misspelledWords.at(i).length() > maxWordLength ? maxWordLength : m_misspelledWords.at(i).length();

I'm surprised this doesn't complain about signed/unsigned either.

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