Fix accessibility issues in custom tab UI

Fix accessibility issues in custom tab UI

Currently long pressing icons in toolbar does not do anything. Also
talkback does not work for back button. These issues are fixed in this
CL.

BUG=504469
Committed: https://crrev.com/32b942d7c5512a09343b91d544e95ac58727f505
Cr-Commit-Position: refs/heads/master@{#337438}

[Ian Wen, 2015-06-29 17:11:14]

ianwen@chromium.org changed reviewers:
+ yusufo@chromium.org

[Yusuf, 2015-06-29 18:03:17]

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:12:
android:contentDescription="@string/accessibility_toolbar_btn_back" />
is this the string we want to have? Or we need a new one?

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:464:
}
else {
return false;
}
here since we may actually want to do something other than showLongPressToast
with a button eventually.

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java:227:
* Shows the content description toast for items on the toolbar, usually after
long clicking.
Lets get rid of mentions about long press since this is not restricted to that
functionally. So just ... on the toolbar.
and rename to showAccessibilityToast

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java:233:
if (stringResId != 0) {
if stringRestId==0 return false;

Then get rid of the indentation below.

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java:406:
return showLongPressToast(v, stringResourceId);
else {
return false;
} here as well

[Ian Wen, 2015-06-29 18:27:42]

Thanks for the comments!

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:12:
android:contentDescription="@string/accessibility_toolbar_btn_back" />
On 2015/06/29 18:03:17, Yusuf wrote:
> is this the string we want to have? Or we need a new one?

This string only says "Go back". Do you want me to create a string saying
something like "Go back to previous screen" or "Go back to client app" or "close
webpage"?

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java:464:
}
On 2015/06/29 18:03:17, Yusuf wrote:
> else {
> return false;
> }
> here since we may actually want to do something other than showLongPressToast
> with a button eventually.

Done.

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java:227:
* Shows the content description toast for items on the toolbar, usually after
long clicking.
On 2015/06/29 18:03:17, Yusuf wrote:
> Lets get rid of mentions about long press since this is not restricted to that
> functionally. So just ... on the toolbar.
> and rename to showAccessibilityToast

Done.

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java:233:
if (stringResId != 0) {
On 2015/06/29 18:03:17, Yusuf wrote:
> if stringRestId==0 return false;
> 
> Then get rid of the indentation below.

Done.

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
(right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java:406:
return showLongPressToast(v, stringResourceId);
On 2015/06/29 18:03:17, Yusuf wrote:
> else {
> return false;
> } here as well

Done.

[Yusuf, 2015-06-29 18:30:08]

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:12:
android:contentDescription="@string/accessibility_toolbar_btn_back" />
On 2015/06/29 18:27:42, Ian Wen wrote:
> On 2015/06/29 18:03:17, Yusuf wrote:
> > is this the string we want to have? Or we need a new one?
> 
> This string only says "Go back". Do you want me to create a string saying
> something like "Go back to previous screen" or "Go back to client app" or
"close
> webpage"?

Yes, lets add a new one and say "Close webpage"

[Ian Wen, 2015-06-30 18:31:22]

ianwen@chromium.org changed reviewers:
+ newt@chromium.org

[Ian Wen, 2015-06-30 18:31:23]

newt@chromium.org: Please review changes in android_chrome_strings.grd

yusufo@ ptal. :P

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/custom_tabs_toolbar.xml (right):

https://codereview.chromium.org/1221693002/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/custom_tabs_toolbar.xml:12:
android:contentDescription="@string/accessibility_toolbar_btn_back" />
On 2015/06/29 18:30:07, Yusuf wrote:
> On 2015/06/29 18:27:42, Ian Wen wrote:
> > On 2015/06/29 18:03:17, Yusuf wrote:
> > > is this the string we want to have? Or we need a new one?
> > 
> > This string only says "Go back". Do you want me to create a string saying
> > something like "Go back to previous screen" or "Go back to client app" or
> "close
> > webpage"?
> 
> Yes, lets add a new one and say "Close webpage"

Done.

[newt (away), 2015-06-30 18:44:58]

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:2021: Close webpage
Should this be "Close tab?" We've already got a string for that BTW.

[Yusuf, 2015-06-30 19:52:30]

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:2021: Close webpage
On 2015/06/30 18:44:58, newt wrote:
> Should this be "Close tab?" We've already got a string for that BTW.

Yeah, I think that also makes sense in this context. Let's use "Close Tab" and
avoid adding then new string.

[Ian Wen, 2015-06-30 20:14:39]

Thanks for the comments. All done.

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/1221693002/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:2021: Close webpage
On 2015/06/30 19:52:30, Yusuf wrote:
> On 2015/06/30 18:44:58, newt wrote:
> > Should this be "Close tab?" We've already got a string for that BTW.
> 
> Yeah, I think that also makes sense in this context. Let's use "Close Tab" and
> avoid adding then new string.

Done.

[Yusuf, 2015-07-01 17:17:40]

lgtm

[Ian Wen, 2015-07-01 17:26:02]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2015-07-01 17:26:59]

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

[commit-bot: I haz the power, 2015-07-01 17:29:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-01 17:29:48]

Exceeded global retry quota

[Ian Wen, 2015-07-06 18:33:26]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2015-07-06 18:33:26]

The patchset sent to the CQ was uploaded after l-g-t-m from yusufo@chromium.org
Link to the patchset: https://codereview.chromium.org/1221693002/#ps100001
(title: "nit")

[commit-bot: I haz the power, 2015-07-06 18:37:33]

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

[commit-bot: I haz the power, 2015-07-06 19:24:47]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-07-06 19:25:47]

Patchset 6 (id:??) landed as
https://crrev.com/32b942d7c5512a09343b91d544e95ac58727f505
Cr-Commit-Position: refs/heads/master@{#337438}