WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
57070
[chromium] Add setters for various client interfaces, and add WbSpellCheckClient
https://bugs.webkit.org/show_bug.cgi?id=57070
Summary
[chromium] Add setters for various client interfaces, and add WbSpellCheckClient
John Abd-El-Malek
Reported
2011-03-24 18:34:41 PDT
[chromium] Add setters for various client interfaces, and add WbSpellCheckClient
Attachments
Patch
(13.72 KB, patch)
2011-03-24 18:35 PDT
,
John Abd-El-Malek
no flags
Details
Formatted Diff
Diff
Patch
(13.56 KB, patch)
2011-03-24 18:38 PDT
,
John Abd-El-Malek
no flags
Details
Formatted Diff
Diff
Patch
(13.84 KB, patch)
2011-03-24 21:31 PDT
,
John Abd-El-Malek
dglazkov
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
John Abd-El-Malek
Comment 1
2011-03-24 18:35:10 PDT
Created
attachment 86865
[details]
Patch
WebKit Review Bot
Comment 2
2011-03-24 18:36:30 PDT
Attachment 86865
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/EditorClientImpl.cpp:899: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
John Abd-El-Malek
Comment 3
2011-03-24 18:36:57 PDT
This is needed as part of the chromium refactoring. A (chrome's) RenderView embedder might not care, or even want to know, about spellchecking, autofill, dev tools etc. By having setters, the code in src\chrome can add them when it gets notified that a RenderView has been created.
John Abd-El-Malek
Comment 4
2011-03-24 18:38:58 PDT
Created
attachment 86867
[details]
Patch
John Abd-El-Malek
Comment 5
2011-03-24 18:39:44 PDT
cc'ing some reviewers, guys whoever wants dibs please go ahead
Ojan Vafai
Comment 6
2011-03-24 18:41:26 PDT
Darin is usually the gatekeeper for Chromium WebKit API changes.
John Abd-El-Malek
Comment 7
2011-03-24 19:17:57 PDT
(In reply to
comment #6
)
> Darin is usually the gatekeeper for Chromium WebKit API changes.
oops, forgot about that. it's been a while :)
John Abd-El-Malek
Comment 8
2011-03-24 21:31:17 PDT
Created
attachment 86878
[details]
Patch
Hajime Morrita
Comment 9
2011-03-25 02:11:36 PDT
Thank you for doing this change! Could you align WebSpellCheckClient method to platform/text/TextCheckingClient if possible? I'd like to move hunspell-based checker code into WebKit from Chromium, and aligning APIs will help.
John Abd-El-Malek
Comment 10
2011-03-25 09:53:10 PDT
(In reply to
comment #9
)
> Thank you for doing this change! > Could you align WebSpellCheckClient method to platform/text/TextCheckingClient if possible? > I'd like to move hunspell-based checker code into WebKit from Chromium, and aligning APIs will help.
I think it's best if these different tasks (refactoring content, and moving the chrome spelling code into webkit) are done separately, since they're orthogonal. It's better if someone who's familiar with the spelling code to do the latter, and to take care of the WebKit API changes etc.
Dimitri Glazkov (Google)
Comment 11
2011-03-25 12:48:29 PDT
Comment on
attachment 86878
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=86878&action=review
I think this is fine, can land after fixes.
> Source/WebKit/chromium/ChangeLog:5 > + [chromium] Add setters for various client interfaces, and add WbSpellCheckClient
WebSpellCheckClient
> Source/WebKit/chromium/src/WebViewImpl.h:119 > + virtual void setWebDevToolsAgentClient(WebDevToolsAgentClient*); > + virtual void setWebAutoFillClient(WebAutoFillClient*); > + virtual void setWebSpellCheckClient(WebSpellCheckClient*);
take out the "Web" in the names for consistency?
John Abd-El-Malek
Comment 12
2011-03-25 13:34:28 PDT
Committed
r81987
: <
http://trac.webkit.org/changeset/81987
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug