"Always Translate" checkbox does not work correctly in TranslateUI2016Q2 bubble |
|||||||||||
Issue descriptionVersion: Version after 52.0.2729.0 OS: Windows opt-in to TranslateUI2016Q2 What steps will reproduce the problem? (1)Visit a page that triggers translate bubble (2)Check and Uncheck "Always do this" several times (3)visit chrome://histograms and look at Translate.BubbleUiEvent What is the expected output? Both ALWAYS_TRANSLATE_CHECKED(4) and ALWAYS_TRANSLATE_UNCHECKED(5) get incremented What do you see instead? Only ALWAYS_TRANSLATE_CHECKED(4) get incremented
,
Jun 17 2016
The problem is worst than just logging. We create two different checkbox in two different view: base view: the newly added "Always do this" advanced view: "Always translate" what happen is when user check/uncheck the one in the base view, the value got used is the one in the advanced view instead- for both logging and the real action.
,
Jun 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf4dd975949936a88973a42df91639cec18d0d2e commit bf4dd975949936a88973a42df91639cec18d0d2e Author: ftang <ftang@chromium.org> Date: Mon Jun 20 21:46:01 2016 Split to use two different ID for the two different checkbox in two different subview. Currently the code always listen to the checkbox created in the advanced view. so even if user click to check/uncheck the box in the before view, the code listen to the value of the checkbox inside the advanced view which was not shown to the user. BUG= 620921 Review-Url: https://codereview.chromium.org/2074983003 Cr-Commit-Position: refs/heads/master@{#400797} [modify] https://crrev.com/bf4dd975949936a88973a42df91639cec18d0d2e/chrome/browser/ui/views/translate/translate_bubble_view.cc [modify] https://crrev.com/bf4dd975949936a88973a42df91639cec18d0d2e/chrome/browser/ui/views/translate/translate_bubble_view.h [modify] https://crrev.com/bf4dd975949936a88973a42df91639cec18d0d2e/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
,
Jun 20 2016
The fix is in. zkoch/groby- I think we should cherry pick this into M52. zkoch - could you propose that?
,
Jun 20 2016
,
Jun 21 2016
Change the summary to reflect the reality.
,
Jun 21 2016
,
Jun 21 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4938a8823ec4e3fe68c789d8dd559281ff061d41 commit 4938a8823ec4e3fe68c789d8dd559281ff061d41 Author: Rachel Blum <groby@google.com> Date: Tue Jun 21 21:51:58 2016 Split to use two different ID for the two different checkbox in two different subview. Currently the code always listen to the checkbox created in the advanced view. so even if user click to check/uncheck the box in the before view, the code listen to the value of the checkbox inside the advanced view which was not shown to the user. BUG= 620921 Review-Url: https://codereview.chromium.org/2074983003 Cr-Commit-Position: refs/heads/master@{#400797} (cherry picked from commit bf4dd975949936a88973a42df91639cec18d0d2e) Review URL: https://codereview.chromium.org/2088053002 . Cr-Commit-Position: refs/branch-heads/2743@{#439} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/4938a8823ec4e3fe68c789d8dd559281ff061d41/chrome/browser/ui/views/translate/translate_bubble_view.cc [modify] https://crrev.com/4938a8823ec4e3fe68c789d8dd559281ff061d41/chrome/browser/ui/views/translate/translate_bubble_view.h [modify] https://crrev.com/4938a8823ec4e3fe68c789d8dd559281ff061d41/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
,
Jun 21 2016
The fix works fine on Win7/64 bit - 53.0.2774.2 Please find the attached screenshot. Both the rows 4 & 5 gets incremented equally under Translate.BubbleUiEvent in chrome://histograms upon checking/un-checking "Always translate" checkbox in the translate bubble.
,
Jun 22 2016
Verified the issue on Windows 7 using chrome latest M52-52.0.2743.49 by following steps mentioned in the comment #0. Observed "Always Translate" checkbox working as expected in TranslateUI2016Q2 bubble. Hence adding TE-Verified label.
,
Jun 29 2016
,
Apr 27 2017
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by ftang@chromium.org
, Jun 17 2016