Summary: | [Chromium] Add two-word misspelling to mock spellchecker | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rouslan Solomakhin <rouslan+webkit> | ||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | groby, jochen, rniwa, rouslan+webkit, tony, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 106815, 108525 | ||||||||||
Attachments: |
|
Description
Rouslan Solomakhin
2013-01-31 10:33:09 PST
Created attachment 185809 [details]
Patch
Comment on attachment 185809 [details] Patch This is the part of https://webkit.org/b/106815 that makesediting/spelling/spelling-exactly-selected-multiple-words pass in Chromium. Comment on attachment 185809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185809&action=review You can use "webkit-patch land-safely" to reupload the patch and forward on the r+. > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:76 > + int maxWordLength = static_cast<int>(stringText.length()) - wordOffset; > + int wordLength; I think you would have fewer casts if you made all the length types unsigned. Conceptually, a negative length doesn't make sense. > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:167 > + m_misspelledWords.append(WTF::String::fromUTF8(misspelledWords[i])); Nit: remove WTF:: > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.h:73 > + Vector<WTF::String> m_misspelledWords; Nit: remove WTF:: Created attachment 185831 [details]
Patch for landing
(In reply to comment #3) > You can use "webkit-patch land-safely" to reupload the patch and forward on the r+. I ran "webkit-patch land-safely", but I think it did not produce the desired effect. It said that do not have EditBugs permission and my login is not listed in committers.py. (In reply to comment #3) > > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:76 > > + int maxWordLength = static_cast<int>(stringText.length()) - wordOffset; > > + int wordLength; > > I think you would have fewer casts if you made all the length types unsigned. Conceptually, a negative length doesn't make sense. As we discussed offline, keeping int types because WTF::String::find() returns int types that can be negative. > > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.cpp:167 > > + m_misspelledWords.append(WTF::String::fromUTF8(misspelledWords[i])); > > Nit: remove WTF:: Done. > > Tools/DumpRenderTree/chromium/TestRunner/src/MockSpellCheck.h:73 > > + Vector<WTF::String> m_misspelledWords; > > Nit: remove WTF:: Done. This patch modifies the actual code, not just the mock code, doesn't it? Comment on attachment 185831 [details]
Patch for landing
Wrong patch, sorry.
Created attachment 185832 [details]
Patch for landing
(In reply to comment #5) > (In reply to comment #3) > > You can use "webkit-patch land-safely" to reupload the patch and forward on the r+. > > I ran "webkit-patch land-safely", but I think it did not produce the desired effect. It said that do not have EditBugs permission and my login is not listed in committers.py. It worked. It added "Reviewed by Tony Chang" and it set cq?. My guess is the editbugs permission was because it tried to assign the bug to you. The committers.py was probably a warning. If you pasted the full error message, that would be helpful to diagnose. It sets cq? because you're not a committer yet. Once you get commit, it will set cq+. (In reply to comment #10) > (In reply to comment #5) > > (In reply to comment #3) > > > You can use "webkit-patch land-safely" to reupload the patch and forward on the r+. > > > > I ran "webkit-patch land-safely", but I think it did not produce the desired effect. It said that do not have EditBugs permission and my login is not listed in committers.py. > > It worked. It added "Reviewed by Tony Chang" and it set cq?. My guess is the editbugs permission was because it tried to assign the bug to you. The committers.py was probably a warning. If you pasted the full error message, that would be helpful to diagnose. > > It sets cq? because you're not a committer yet. Once you get commit, it will set cq+. Reading your explanation makes me think that the behavior was correct. Here is the command output, just to make sure: $ ./Tools/Scripts/webkit-patch land-safely Fetching: https://bugs.webkit.org/show_bug.cgi?id=108498&ctype=xml&excludefield=attachmentdata No reviewed patches on bug 108498, cannot infer reviewer. Failed to guess reviewer from bug 108498 and --reviewer= not provided. Not updating ChangeLogs with reviewer. Fetching: https://bugs.webkit.org/show_bug.cgi?id=108498&ctype=xml&excludefield=attachmentdata Fetching: https://bugs.webkit.org/show_bug.cgi?id=108498&ctype=xml&excludefield=attachmentdata Logging in as rouslan+webkit@chromium.org... Assigning bug 108498 to rouslan+webkit@chromium.org Failed to assign bug to you (can't find assigned_to) control. Do you have EditBugs privileges at bugs.webkit.org? https://bugs.webkit.org/userprefs.cgi?tab=permissions If not, you should email webkit-committers@lists.webkit.org or ask in #webkit for someone to add EditBugs to your bugs.webkit.org account. Adding patch "Patch for landing" to https://bugs.webkit.org/show_bug.cgi?id=108498 Your Bugzilla login is not listed in committers.py. Uploading with cq? instead of cq+ Comment on attachment 185832 [details] Patch for landing Clearing flags on attachment: 185832 Committed r141471: <http://trac.webkit.org/changeset/141471> All reviewed patches have been landed. Closing bug. |