New issue
Advanced search Search tips

Issue 890465 link

Starred by 2 users
Status: Fixed
Owner:
sapz...@gmail.com
Closed: Oct 31
Cc:
siyua@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Update duplicate code of tooltipt text and accessible name in views::ImageView and views::Button

Project Member Reported by siyua@chromium.org, Sep 28 
Change to either:

(1) Calling GetAccessibleName() in GetAccessibleNodeData():
      node_data->SetName(GetAccessibleName());
    And changing GetAccessibleName to do:
      return accessible_name_.empty() ? tooltip_text_ : accessible_name_;
(2) Changing GetAccessibleNodeData to do:
      node_data->SetName(accessible_name_.empty() ? tooltip_text_ : accessible_name_);

Comment 1 by sapz...@gmail.com, Oct 1 

Can I take this issue if there is no one working on this?

Comment 2 by siyua@chromium.org, Oct 1 

Sure. No one is working on this right now so feel free to take it. Please send to msw@chromium.org for review when its finished. And please refer to this CL for more context: https://chromium-review.googlesource.com/c/chromium/src/+/1244888. Thanks!
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11 

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

commit 07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77
Author: Jongheon Kim <sapzape@gmail.com>
Date: Thu Oct 11 12:24:19 2018

Update GetAccessibleName() in view::ImageView and view::Button

This patch is updated duplicate code of tooltip text and accesible name

Bug:  890465 
Change-Id: I4ed08c50a91a216de5d647d0d137e8927129b6af
Reviewed-on: https://chromium-review.googlesource.com/c/1256308
Commit-Queue: Jongheon Kim <sapzape@gmail.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598729}
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/accessibility/dictation_button_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/accessibility/select_to_speak_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/message_center/notification_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/network/network_list.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/tray/actionable_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/tray/hover_highlight_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/tray/tray_info_label_unittest.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/tray/tray_item_more.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/unified/notification_counter_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/unified/user_chooser_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/user/user_card_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ash/system/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/ime/ime_window_frame_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/payments/payment_request_views_util.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ui/message_center/views/notification_header_view.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ui/views/controls/button/button.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ui/views/controls/button/button.h
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ui/views/controls/image_view_base.cc
[modify] https://crrev.com/07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77/ui/views/controls/image_view_base.h
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 11 

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

commit 03cb5c6ae11ab562bc364560972f74efdd66a761
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Thu Oct 11 13:10:07 2018

Revert "Update GetAccessibleName() in view::ImageView and view::Button"

This reverts commit 07f45e13abeab9b8cf97e7a45aecd5d37d6d7d77.

Reason for revert: broke compilation on Win
https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/38615
../../chrome/browser/ui/views/policy/enterprise_startup_dialog_view.cc(171,15):  error: no member named 'SetTooltipText' in 'views::ImageView'; did you mean 'set_tooltip_text'?
  logo_image->SetTooltipText(
              ^~~~~~~~~~~~~~
              set_tooltip_text

Original change's description:
> Update GetAccessibleName() in view::ImageView and view::Button
> 
> This patch is updated duplicate code of tooltip text and accesible name
> 
> Bug:  890465 
> Change-Id: I4ed08c50a91a216de5d647d0d137e8927129b6af
> Reviewed-on: https://chromium-review.googlesource.com/c/1256308
> Commit-Queue: Jongheon Kim <sapzape@gmail.com>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598729}

TBR=xiyuan@chromium.org,stevenjb@chromium.org,msw@chromium.org,jinho.bang@samsung.com,sapzape@gmail.com

Change-Id: Icd106f1c5e23a1a2f9322f867688f44809bd28b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  890465 
Reviewed-on: https://chromium-review.googlesource.com/c/1275856
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598734}
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/accessibility/dictation_button_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/accessibility/select_to_speak_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/message_center/notification_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/network/network_list.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/tray/actionable_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/tray/hover_highlight_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/tray/tray_info_label_unittest.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/tray/tray_item_more.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/unified/notification_counter_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/unified/user_chooser_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/user/user_card_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ash/system/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/ime/ime_window_frame_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/payments/payment_request_views_util.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ui/message_center/views/notification_header_view.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ui/views/controls/button/button.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ui/views/controls/button/button.h
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ui/views/controls/image_view_base.cc
[modify] https://crrev.com/03cb5c6ae11ab562bc364560972f74efdd66a761/ui/views/controls/image_view_base.h
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 21 

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

commit 448b9f1f98e887c3a96564d0fd346b60f8c49212
Author: Jongheon Kim <sapzape@gmail.com>
Date: Sun Oct 21 03:51:26 2018

Re-update GetAccessibleName() in view::ImageView and view::Button

This patch is updated duplicate code of tooltip text and accesible name

Bug:  890465 
Change-Id: I74ba045aab1f5668db0e10c279c8ce99f993238d
Reviewed-on: https://chromium-review.googlesource.com/c/1278076
Commit-Queue: Jongheon Kim <sapzape@gmail.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601416}
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/app_list/views/search_result_answer_card_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/accessibility/dictation_button_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/accessibility/select_to_speak_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/message_center/notification_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/network/network_list.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/tray/actionable_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/tray/hover_highlight_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/tray/tray_info_label_unittest.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/tray/tray_item_more.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/unified/notification_counter_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/unified/user_chooser_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/user/user_card_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ash/system/virtual_keyboard/virtual_keyboard_tray.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/autofill/card_unmask_prompt_views.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/ime/ime_window_frame_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/payments/payment_request_views_util.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/chrome/browser/ui/views/policy/enterprise_startup_dialog_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ui/message_center/views/notification_header_view.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ui/views/controls/button/button.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ui/views/controls/button/button.h
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ui/views/controls/image_view_base.cc
[modify] https://crrev.com/448b9f1f98e887c3a96564d0fd346b60f8c49212/ui/views/controls/image_view_base.h

Comment 6 by siyua@chromium.org, Oct 31

Cc: siyua@chromium.org
Owner: sapz...@gmail.com
Status: Fixed (was: Assigned)
Hi Jongheon, thanks for taking this issue. Seems this is resolved with your CL. Will change owner to you and mark this as fixed.
►

Sign in to add a comment