Don't re-focus Settings window when opening IME options

Don't re-focus Settings window when opening IME options

These five lines just cause the current browser window to be shown after
the tab with the IME options has already been focused and activated in
another window.

BUG=409270
R=stevenjb@chromium.org,shuchen@chromium.org

Committed: https://crrev.com/9bff5bdca927eb974da275b0353d3eba39d0b76d
Cr-Commit-Position: refs/heads/master@{#296537}

[michaelpg, 2014-09-24 00:07:24]

shuchen: I think this code is unnecessary. I know it was a while ago, but let me
know if you remember a reason that we needed it.

Original change:
https://codereview.chromium.org/136693012/diff/490001/chrome/browser/ui/webui...

Thanks.

[Shu Chen, 2014-09-24 00:56:35]

LGTM.

The lines I added were just copied from somewhere, and at that time (settings
page was a tab) I thought it could make sure to bring the IME options page to
front.

Thanks for the fix!

[stevenjb, 2014-09-24 16:12:53]

lgtm w/ comment

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:245:
browser->OpenURL(params);
nit: It looks to me like this will allways be an extension settings URL,
correct?
We might want to add a TODO here to use a special helper method for opening
extension settings pages so that if/when we change how we do that this will do
the right thing. We should include 'chrome-extension:' in the comment so that a
grep will find it.
+kalman@ just FYI

Also, does removing these lines affect the behavior on non-chromeos (i.e.
settings in a tab)? They seem unnecessary, and this should definitely be handled
in OpenURL(), not here, but we should confirm that.

[michaelpg, 2014-09-24 21:23:06]

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:245:
browser->OpenURL(params);
On 2014/09/24 16:12:53, stevenjb wrote:
> nit: It looks to me like this will allways be an extension settings URL,
> correct?

I think so but I'm not positive.

> We might want to add a TODO here to use a special helper method for opening
> extension settings pages so that if/when we change how we do that this will do
> the right thing. We should include 'chrome-extension:' in the comment so that
a
> grep will find it.
> +kalman@ just FYI

Probably a good idea. I will let kalman@ or shuchen@ decide whether to do so and
whom to assign the TODO to just so I can commit this before the branch happens.

> 
> Also, does removing these lines affect the behavior on non-chromeos (i.e.
> settings in a tab)? They seem unnecessary, and this should definitely be
handled
> in OpenURL(), not here, but we should confirm that.

This is Chrome OS-only. Like it literally doesn't compile on other platforms,
right?

[michaelpg, 2014-09-24 21:23:18]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2014-09-24 21:23:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/599663003/1

[commit-bot: I haz the power, 2014-09-24 22:01:35]

Committed patchset #1 (id:1) as f8ce9a823e540cfa9b8e951e72a55f2b5366b9f8

[commit-bot: I haz the power, 2014-09-24 22:02:25]

Patchset 1 (id:??) landed as
https://crrev.com/9bff5bdca927eb974da275b0353d3eba39d0b76d
Cr-Commit-Position: refs/heads/master@{#296537}

[stevenjb, 2014-09-25 15:49:42]

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
(right):

https://codereview.chromium.org/599663003/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc:245:
browser->OpenURL(params);
On 2014/09/24 21:23:06, michaelpg wrote:

> This is Chrome OS-only. Like it literally doesn't compile on other platforms,
> right?

Ah, yes, you're correct, I didn't notice this was
cros_language_options_handler.cc and not just language_options_handler.cc.
Confusing.