Bug 41244 – [chromium]Remove deprecated code related to input method.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[James Su]

This patch just removes some deprecated code. No functionality change.
Please refer to: https://bugs.webkit.org/show_bug.cgi?id=40608

[James Su]

Created attachment 59822 [details]
The patch for review.

The patch for review.

[WebKit Review Bot]

Attachment 59822 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3350023

[Tony Chang]

The build on chromium failed.  Looks like there are still references to WebCompositionCommand.h (probably the include statement itself?).  Should be fine once you get it to pass the EWS bots.

[Jian Li]

Comment on attachment 59822 [details]
The patch for review.

You need to remove the file from WebKit/chromium/WebKit.gyp. Please make sure you build and test the Chromium.

[James Su]

Created attachment 59910 [details]
Fix build issue.

[Jian Li]

Comment on attachment 59910 [details]
Fix build issue.

Do you want to have the reviewer to add your patch to the commit queue? If so, please set the commit-queue flag from empty to "?".

WebKit/chromium/ChangeLog:18
 +          * tests/PopupMenuTest.cpp:
Please also mention here why you need to change this file to override the default implementations since the main description only says to remove the deprecated code.

[James Su]

(In reply to comment #6)
> (From update of attachment 59910 [details])
> Do you want to have the reviewer to add your patch to the commit queue? If so, please set the commit-queue flag from empty to "?".
> 
> WebKit/chromium/ChangeLog:18
>  +          * tests/PopupMenuTest.cpp:
> Please also mention here why you need to change this file to override the default implementations since the main description only says to remove the deprecated code.

I should have made this change in the patch of bug 40608. These new methods are pure virtual, so they needs to be overridden here. But as this TestWebWidget class is not used anywhere, it's actually won't break any test without this change.

[Jian Li]

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 59910 [details] [details])
> > Do you want to have the reviewer to add your patch to the commit queue? If so, please set the commit-queue flag from empty to "?".
> > 
> > WebKit/chromium/ChangeLog:18
> >  +          * tests/PopupMenuTest.cpp:
> > Please also mention here why you need to change this file to override the default implementations since the main description only says to remove the deprecated code.
> 
> I should have made this change in the patch of bug 40608. These new methods are pure virtual, so they needs to be overridden here. But as this TestWebWidget class is not used anywhere, it's actually won't break any test without this change.

It would be better to mention all the changes made in the patch. It might be as simple as adding the following line to the description.
  Also update TestPopupMenuClient to add missing implementations.

Also, what I mean is that you need to set both review flag and commit-queue flag when you submit a new patch for review if you want to use the commit queue to commit your patch.

[James Su]

Created attachment 59928 [details]
Update ChangeLog to mention the change in TestPopupMenu.cpp

[Jian Li]

Comment on attachment 59928 [details]
Update ChangeLog to mention the change in TestPopupMenu.cpp

r=me

[WebKit Commit Bot]

Comment on attachment 59928 [details]
Update ChangeLog to mention the change in TestPopupMenu.cpp

Clearing flags on attachment: 59928

Committed r62039: <http://trac.webkit.org/changeset/62039>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.