Bug 68532 - [WK2] Realign TextChecker implementation of windows port to common independent part for all ports
Summary: [WK2] Realign TextChecker implementation of windows port to common independen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 68547 68548
  Show dependency treegraph
 
Reported: 2011-09-21 07:29 PDT by Ravi Phaneendra Kasibhatla
Modified: 2011-10-06 14:33 PDT (History)
8 users (show)

See Also:


Attachments
Moving TextChecker from win port to all ports (83.59 KB, patch)
2011-09-21 08:07 PDT, Ravi Phaneendra Kasibhatla
no flags Details | Formatted Diff | Diff
GTK port implementation for Text Checker (10.89 KB, patch)
2011-09-21 08:08 PDT, Ravi Phaneendra Kasibhatla
gustavo: commit-queue-
Details | Formatted Diff | Diff
App changes for TextChecker (12.28 KB, patch)
2011-09-21 08:10 PDT, Ravi Phaneendra Kasibhatla
gustavo.noronha: commit-queue-
Details | Formatted Diff | Diff
Moving TextChecker implementation from win port to common files (81.26 KB, patch)
2011-09-21 23:27 PDT, Ravi Phaneendra Kasibhatla
andersca: review-
Details | Formatted Diff | Diff
Make window TextChecker implementation generic for all platforms (81.66 KB, patch)
2011-10-04 11:48 PDT, Ravi Phaneendra Kasibhatla
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ravi Phaneendra Kasibhatla 2011-09-21 07:29:13 PDT
Implement the TextChecker for WebKit2 GTK port.
Further enhancements:
- Showing the suggestions for misspelled words in the context menu. Currently it just shows the misspelled words.
Comment 1 Ravi Phaneendra Kasibhatla 2011-09-21 08:07:58 PDT
Created attachment 108161 [details]
Moving TextChecker from win port to all ports

Patch moves all the TextChecker feature implementation classes except TextChecker class from win port to common. All the files are completely platform independent and can be reused across all ports. Patch also corrects the windows builds files related to change of location of the files.
Comment 2 Ravi Phaneendra Kasibhatla 2011-09-21 08:08:58 PDT
Created attachment 108162 [details]
GTK port implementation for Text Checker

Patch includes the implementation of TextChecker class functions for GTK as well as makefile changes for GTK.
Comment 3 Ravi Phaneendra Kasibhatla 2011-09-21 08:10:23 PDT
Created attachment 108163 [details]
App changes for TextChecker 

App changes for testing the TextChecker feature. It currently addresses getting the words from enchant library. Doesn't support adding the suggestions to the Context Menu for usage.
Comment 4 WebKit Review Bot 2011-09-21 08:15:10 PDT
Attachment 108163 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/..." exit_code: 1

Last 3072 characters of output:
claration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:738:  Declaration has space between * and variable name in EnchantDict* dict  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:750:  Declaration has space between * and variable name in BrowserWindow* window  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:751:  Declaration has space between * and variable name in GSList* dicts  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:756:  Declaration has space between * and variable name in char* string  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:758:  Declaration has space between * and variable name in PangoLanguage* language  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:759:  Declaration has space between * and variable name in PangoLogAttr* attrs  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:782:  Declaration has space between * and variable name in gchar* cstart  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:784:  Declaration has space between * and variable name in gchar* word  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:789:  Declaration has space between * and variable name in EnchantDict* dict  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:813:  Declaration has space between * and variable name in BrowserWindow* window  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:814:  Declaration has space between * and variable name in GSList* dicts  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:820:  Declaration has space between * and variable name in char* string  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:827:  Declaration has space between * and variable name in EnchantDict* dict  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:852:  Declaration has space between * and variable name in BrowserWindow* window  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:853:  Declaration has space between * and variable name in GSList* dicts  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:858:  Declaration has space between * and variable name in char* string  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:862:  Declaration has space between * and variable name in EnchantDict* dict  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:872:  Declaration has space between * and variable name in BrowserWindow* window  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:873:  Declaration has space between * and variable name in GSList* dicts  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:878:  Declaration has space between * and variable name in char* string  [whitespace/declaration] [3]
Tools/MiniBrowser/gtk/BrowserWindow.c:882:  Declaration has space between * and variable name in EnchantDict* dict  [whitespace/declaration] [3]
Total errors found: 32 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Adam Roben (:aroben) 2011-09-21 08:24:32 PDT
Comment on attachment 108161 [details]
Moving TextChecker from win port to all ports

View in context: https://bugs.webkit.org/attachment.cgi?id=108161&action=review

Anders and/or Sam should probably review this.

> Source/WebKit2/ChangeLog:75
> +        * Shared/API/c/WKBase.h: Add WKGrammarDetailRef & WKTextCheckerRef types.
> +        * Shared/API/c/win/WKBaseWin.h: Remove WKGrammarDetailRef & WKTextCheckerRef types.
> +        * Shared/APIObject.h:
> +        * UIProcess/API/C/WKAPICast.h: Added mapping for WKTextCheckerRef & WKGrammarDetailRef.
> +        (WebKit::toAPI):
> +        * UIProcess/API/C/WKGrammarDetail.cpp: Renamed from Source/WebKit2/UIProcess/API/C/win/WKGrammarDetail.cpp.
> +        (WKGrammarDetailGetTypeID):
> +        (WKGrammarDetailCreate):
> +        (WKGrammarDetailGetLocation):
> +        (WKGrammarDetailGetLength):
> +        (WKGrammarDetailCopyGuesses):
> +        (WKGrammarDetailCopyUserDescription):
> +        * UIProcess/API/C/WKGrammarDetail.h: Renamed from Source/WebKit2/UIProcess/API/C/win/WKGrammarDetail.h.
> +        * UIProcess/API/C/WKTextChecker.cpp: Renamed from Source/WebKit2/UIProcess/API/C/win/WKTextChecker.cpp.
> +        (WKTextCheckerSetClient):
> +        (WKTextCheckerContinuousSpellCheckingEnabledStateChanged):
> +        (WKTextCheckerGrammarCheckingEnabledStateChanged):
> +        (WKTextCheckerCheckSpelling):
> +        (WKTextCheckerChangeSpellingToWord):
> +        * UIProcess/API/C/WKTextChecker.h: Renamed from Source/WebKit2/UIProcess/API/C/win/WKTextChecker.h.
> +        * UIProcess/API/C/win/WKAPICastWin.h: Removed mapping for WKTextCheckerRef & WKGrammarDetailRef.
> +        * UIProcess/WebGrammarDetail.cpp: Renamed from Source/WebKit2/UIProcess/win/WebGrammarDetail.cpp.
> +        (WebKit::WebGrammarDetail::create):
> +        (WebKit::WebGrammarDetail::WebGrammarDetail):
> +        (WebKit::WebGrammarDetail::guesses):
> +        * UIProcess/WebGrammarDetail.h: Renamed from Source/WebKit2/UIProcess/win/WebGrammarDetail.h.
> +        (WebKit::WebGrammarDetail::location):
> +        (WebKit::WebGrammarDetail::length):
> +        (WebKit::WebGrammarDetail::userDescription):
> +        (WebKit::WebGrammarDetail::grammarDetail):
> +        (WebKit::WebGrammarDetail::type):
> +        * UIProcess/WebTextChecker.cpp: Renamed from Source/WebKit2/UIProcess/win/WebTextChecker.cpp.
> +        (WebKit::WebTextChecker::shared):
> +        (WebKit::WebTextChecker::WebTextChecker):
> +        (WebKit::WebTextChecker::setClient):
> +        (WebKit::updateStateForAllWebProcesses):
> +        (WebKit::WebTextChecker::continuousSpellCheckingEnabledStateChanged):
> +        (WebKit::WebTextChecker::grammarCheckingEnabledStateChanged):
> +        (WebKit::WebTextChecker::checkSpelling):
> +        (WebKit::WebTextChecker::changeSpellingToWord):
> +        * UIProcess/WebTextChecker.h: Renamed from Source/WebKit2/UIProcess/win/WebTextChecker.h.
> +        (WebKit::WebTextChecker::client):
> +        (WebKit::WebTextChecker::type):
> +        * UIProcess/WebTextCheckerClient.cpp: Renamed from Source/WebKit2/UIProcess/win/WebTextCheckerClient.cpp.
> +        (WebKit::WebTextCheckerClient::continuousSpellCheckingAllowed):
> +        (WebKit::WebTextCheckerClient::continuousSpellCheckingEnabled):
> +        (WebKit::WebTextCheckerClient::setContinuousSpellCheckingEnabled):
> +        (WebKit::WebTextCheckerClient::grammarCheckingEnabled):
> +        (WebKit::WebTextCheckerClient::setGrammarCheckingEnabled):
> +        (WebKit::WebTextCheckerClient::uniqueSpellDocumentTag):
> +        (WebKit::WebTextCheckerClient::closeSpellDocumentWithTag):
> +        (WebKit::WebTextCheckerClient::checkSpellingOfString):
> +        (WebKit::WebTextCheckerClient::checkGrammarOfString):
> +        (WebKit::WebTextCheckerClient::spellingUIIsShowing):
> +        (WebKit::WebTextCheckerClient::toggleSpellingUIIsShowing):
> +        (WebKit::WebTextCheckerClient::updateSpellingUIWithMisspelledWord):
> +        (WebKit::WebTextCheckerClient::updateSpellingUIWithGrammarString):
> +        (WebKit::WebTextCheckerClient::guessesForWord):
> +        (WebKit::WebTextCheckerClient::learnWord):
> +        (WebKit::WebTextCheckerClient::ignoreWord):
> +        * UIProcess/WebTextCheckerClient.h: Renamed from Source/WebKit2/UIProcess/win/WebTextCheckerClient.h.
> +        * win/WebKit2.vcproj: Corrected paths for renamed files.
> +        * win/WebKit2Generated.make: Corrected paths for renamed files.

I really like the comments you added for the changes you made!

I think it is far more readable to delete the function names for files that were simply renamed. They're just adding noise, not information.
Comment 6 Gustavo Noronha (kov) 2011-09-21 08:39:35 PDT
Comment on attachment 108162 [details]
GTK port implementation for Text Checker

Attachment 108162 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9772748
Comment 7 Collabora GTK+ EWS bot 2011-09-21 08:41:17 PDT
Comment on attachment 108163 [details]
App changes for TextChecker 

Attachment 108163 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9764794
Comment 8 Ravi Phaneendra Kasibhatla 2011-09-21 10:35:12 PDT
(In reply to comment #6)
> (From update of attachment 108162 [details])
> Attachment 108162 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/9772748
kov,
Attachments 108162 & 108163 have a dependency on https://bugs.webkit.org/attachment.cgi?id=108161.
Comment 9 Ravi Phaneendra Kasibhatla 2011-09-21 11:17:46 PDT
Comment on attachment 108162 [details]
GTK port implementation for Text Checker

Obsolete the patch since it is captured in https://bugs.webkit.org/show_bug.cgi?id=68548
Comment 10 Ravi Phaneendra Kasibhatla 2011-09-21 11:18:14 PDT
Comment on attachment 108163 [details]
App changes for TextChecker 

Obsolete the patch since it is captured in https://bugs.webkit.org/show_bug.cgi?id=68547
Comment 11 Ravi Phaneendra Kasibhatla 2011-09-21 23:27:03 PDT
Created attachment 108279 [details]
Moving TextChecker implementation from win port to common files

Modified ChangeLog as per aroben comments.
Comment 12 Ravi Phaneendra Kasibhatla 2011-09-27 10:51:19 PDT
andersca, sam:
It would be great if you can update with comments if any?
Comment 13 Anders Carlsson 2011-10-04 09:52:43 PDT
Comment on attachment 108279 [details]
Moving TextChecker implementation from win port to common files

Patch looks fine, but please upload a new patch that applies so we can make sure it doesn't break any builds.
Comment 14 Ravi Phaneendra Kasibhatla 2011-10-04 10:15:10 PDT
Since the Text Checker implementation of windows port is completely platform independent, moving all related files to common folders. Changing the title to reflect the same.
Comment 15 Ravi Phaneendra Kasibhatla 2011-10-04 11:48:32 PDT
Created attachment 109660 [details]
Make window TextChecker implementation generic for all platforms
Comment 16 WebKit Review Bot 2011-10-06 14:33:19 PDT
Comment on attachment 109660 [details]
Make window TextChecker implementation generic for all platforms

Clearing flags on attachment: 109660

Committed r96858: <http://trac.webkit.org/changeset/96858>
Comment 17 WebKit Review Bot 2011-10-06 14:33:29 PDT
All reviewed patches have been landed.  Closing bug.