New issue
Advanced search Search tips

Issue 750344 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Add accessibility ID for Cancel and Done button on Translate Language Picker

Project Member Reported by liaoyuke@chromium.org, Jul 28 2017

Issue description

Currently, the test uses the following matchers to find the cancel and done button on language picker:

grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(@"Cancel"),
                    grey_userInteractionEnabled(), nil);

This is not bullet proof and may break any time, and a better solution is to add accessibility ids to the two buttons for testing.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9e3aebde0d87c2ad14044f440ddc6513dfe84dd

commit b9e3aebde0d87c2ad14044f440ddc6513dfe84dd
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Tue Aug 01 02:46:42 2017

Add AX ID for Cancel and Done button on Translate Language Picker.

Currently, the test uses the following matchers to find the cancel and
done button on language picker:

grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(@"Cancel"),
                    grey_userInteractionEnabled(), nil);

This is not bullet proof and may break any time, and this CL fixes the
issue by adding AX ID to both the cancel and done buttons.

Bug:  750344 
Change-Id: I4fda45961b684b877adf29a5bf0fee6e03f1ba8d
Reviewed-on: https://chromium-review.googlesource.com/594831
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490840}
[modify] https://crrev.com/b9e3aebde0d87c2ad14044f440ddc6513dfe84dd/ios/chrome/browser/translate/before_translate_infobar_controller.h
[modify] https://crrev.com/b9e3aebde0d87c2ad14044f440ddc6513dfe84dd/ios/chrome/browser/translate/before_translate_infobar_controller.mm
[modify] https://crrev.com/b9e3aebde0d87c2ad14044f440ddc6513dfe84dd/ios/chrome/browser/translate/translate_egtest.mm

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6163e5017ad9a35379900ea7eb7453edcda8a8f7

commit 6163e5017ad9a35379900ea7eb7453edcda8a8f7
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Aug 02 10:00:07 2017

Revert "Add AX ID for Cancel and Done button on Translate Language Picker."

This reverts commit b9e3aebde0d87c2ad14044f440ddc6513dfe84dd.

Reason for revert: I think this is breaking TranslateTestCase.testLanguageDetectionInfobar. I cannot reproduce the error locally.

Original change's description:
> Add AX ID for Cancel and Done button on Translate Language Picker.
> 
> Currently, the test uses the following matchers to find the cancel and
> done button on language picker:
> 
> grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(@"Cancel"),
>                     grey_userInteractionEnabled(), nil);
> 
> This is not bullet proof and may break any time, and this CL fixes the
> issue by adding AX ID to both the cancel and done buttons.
> 
> Bug:  750344 
> Change-Id: I4fda45961b684b877adf29a5bf0fee6e03f1ba8d
> Reviewed-on: https://chromium-review.googlesource.com/594831
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#490840}

TBR=eugenebut@chromium.org,liaoyuke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  750344 
Change-Id: Ibb4f68af8e0c502000d063d6b19ff185fc1b1a42
Reviewed-on: https://chromium-review.googlesource.com/597627
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491327}
[modify] https://crrev.com/6163e5017ad9a35379900ea7eb7453edcda8a8f7/ios/chrome/browser/translate/before_translate_infobar_controller.h
[modify] https://crrev.com/6163e5017ad9a35379900ea7eb7453edcda8a8f7/ios/chrome/browser/translate/before_translate_infobar_controller.mm
[modify] https://crrev.com/6163e5017ad9a35379900ea7eb7453edcda8a8f7/ios/chrome/browser/translate/translate_egtest.mm

Status: Assigned (was: Fixed)
CL was reverted
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be3e31b73c61fc224c3e17bc0f3fab42665f6178

commit be3e31b73c61fc224c3e17bc0f3fab42665f6178
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Thu Aug 03 17:18:18 2017

Reland"Add AX ID for Cancel and Done button on Translate Language Picker."

This reverts commit 6163e5017ad9a35379900ea7eb7453edcda8a8f7.

The original CL's test fails on iPhone 5s, iOS 9.0 because the 
accessibility identifiers of the buttons are not set even though 
setAccessibilityIdentifier is called explicitly in the code, and it
looks like there is nothing we can do about it. This CL fixes the issue
by keeping using accessbilityLabel on iOS 9 and iOS 10, and switch to
use accessibilityID on iOS 11, and will clean this code up once the
support for iOS 10 is dropped.

Original change's description:
> Add AX ID for Cancel and Done button on Translate Language Picker.
> 
> Currently, the test uses the following matchers to find the cancel and
> done button on language picker:
> 
> grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(@"Cancel"),
>                     grey_userInteractionEnabled(), nil);
> 
> This is not bullet proof and may break any time, and this CL fixes the
> issue by adding AX ID to both the cancel and done buttons.

Bug:  750344 
Change-Id: I57d1c716118049516d93853da77c25d85385aae0
Reviewed-on: https://chromium-review.googlesource.com/599124
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491774}
[modify] https://crrev.com/be3e31b73c61fc224c3e17bc0f3fab42665f6178/ios/chrome/browser/translate/before_translate_infobar_controller.h
[modify] https://crrev.com/be3e31b73c61fc224c3e17bc0f3fab42665f6178/ios/chrome/browser/translate/before_translate_infobar_controller.mm
[modify] https://crrev.com/be3e31b73c61fc224c3e17bc0f3fab42665f6178/ios/chrome/browser/translate/translate_egtest.mm

Status: Fixed (was: Assigned)

Sign in to add a comment