Translate (Win): add after infobar UI for server side language detection

Translate (Win): add after infobar UI for server side language detection

After enabling context menu to translate unknown language, Chrome needs
to modify infobar UX for Chrome Translate.

This change fixes Win UI.

BUG=172593
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202398

[Takashi Toyoshima, 2013-05-21 08:31:48]

Hi sky,

Can you review this views UI change?
This change is needed once https://chromiumcodereview.appspot.com/15295004/ is
landed.

FYI,
for linux; https://chromiumcodereview.appspot.com/15297006/
for mac; https://chromiumcodereview.appspot.com/15521005/

[sky, 2013-05-21 15:05:07]

sky->pkasting
Peter wanted to see all infobar changes.

[Peter Kasting, 2013-05-22 23:30:00]

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
File chrome/browser/ui/views/infobars/after_translate_infobar.cc (right):

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:30:
TranslateInfoBarDelegate::kNoIndex;
Nit: Break after first '=' instead

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:75:
revert_button_->SetBounds(last_right_bounds + kButtonInLabelSpacing,
Nit: You can omit the temporary by doing this:

  revert_button_->SetBounds(
      (label_3_ ? label_3_ : label_2_)->bounds().right() +
          kButtonInLabelSpacing, OffsetY(revert_button_size),
      revert_button_size.width(), revert_button_size.height());

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:93:
TranslateInfoBarDelegate::GetAfterTranslateWithAutoStrings(&strings);
You didn't include these changes in this CL.

I suggest either having just one GetAfterTranslateStrings() function that takes
a bool |autodetemined_source_language|, or else having this function be called
GetAfterTranslateStringsForAutodeterminedSourceLanguage().

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:113:
AddChildView(swapped_language_buttons_ || translate_with_auto_ ?
Nit: Put parens around the initial ||'d part.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
File chrome/browser/ui/views/infobars/after_translate_infobar.h (right):

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.h:60: bool
translate_with_auto_;
I'm not sure what "translate with auto" is supposed to mean.  Perhaps this could
be "autodetermined_source_language_"?

[Takashi Toyoshima, 2013-05-23 14:13:27]

Done.

This change still depends on https://codereview.chromium.org/15297006/ being in
CQ.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
File chrome/browser/ui/views/infobars/after_translate_infobar.cc (right):

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:30:
TranslateInfoBarDelegate::kNoIndex;
On 2013/05/22 23:30:00, Peter Kasting wrote:
> Nit: Break after first '=' instead

Done.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:75:
revert_button_->SetBounds(last_right_bounds + kButtonInLabelSpacing,
On 2013/05/22 23:30:00, Peter Kasting wrote:
> Nit: You can omit the temporary by doing this:
> 
>   revert_button_->SetBounds(
>       (label_3_ ? label_3_ : label_2_)->bounds().right() +
>           kButtonInLabelSpacing, OffsetY(revert_button_size),
>       revert_button_size.width(), revert_button_size.height());

Done.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:93:
TranslateInfoBarDelegate::GetAfterTranslateWithAutoStrings(&strings);
On 2013/05/22 23:30:00, Peter Kasting wrote:
> You didn't include these changes in this CL.
> 
> I suggest either having just one GetAfterTranslateStrings() function that
takes
> a bool |autodetemined_source_language|, or else having this function be called
> GetAfterTranslateStringsForAutodeterminedSourceLanguage().

Done.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:113:
AddChildView(swapped_language_buttons_ || translate_with_auto_ ?
On 2013/05/22 23:30:00, Peter Kasting wrote:
> Nit: Put parens around the initial ||'d part.

Done.

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
File chrome/browser/ui/views/infobars/after_translate_infobar.h (right):

https://chromiumcodereview.appspot.com/15376008/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/infobars/after_translate_infobar.h:60: bool
translate_with_auto_;
On 2013/05/22 23:30:00, Peter Kasting wrote:
> I'm not sure what "translate with auto" is supposed to mean.  Perhaps this
could
> be "autodetermined_source_language_"?

Done.

[Peter Kasting, 2013-05-24 01:31:16]

LGTM

https://chromiumcodereview.appspot.com/15376008/diff/27001/chrome/browser/ui/...
File chrome/browser/ui/views/infobars/after_translate_infobar.cc (right):

https://chromiumcodereview.appspot.com/15376008/diff/27001/chrome/browser/ui/...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:95: if
(autodetermined_source_language_)
Nit: Simpler:

  DCHECK_EQ(autodetermined_source_language_ ? 2U : 3U, strings.size());

[Takashi Toyoshima, 2013-05-27 07:01:08]

thank you.

https://chromiumcodereview.appspot.com/15376008/diff/27001/chrome/browser/ui/...
File chrome/browser/ui/views/infobars/after_translate_infobar.cc (right):

https://chromiumcodereview.appspot.com/15376008/diff/27001/chrome/browser/ui/...
chrome/browser/ui/views/infobars/after_translate_infobar.cc:95: if
(autodetermined_source_language_)
On 2013/05/24 01:31:16, Peter Kasting wrote:
> Nit: Simpler:
> 
>   DCHECK_EQ(autodetermined_source_language_ ? 2U : 3U, strings.size());

Done.

[commit-bot: I haz the power, 2013-05-27 07:01:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/15376008/33001

[commit-bot: I haz the power, 2013-05-27 12:51:58]

Change committed as 202398