Focus order is wrong on non-arc notification |
|||||||||
Issue descriptionExpected reverse focus order [next notification] 1. close button (if available) 2. settings button (if available) 3. buttons (if available) 4. Whole notification [previous notification] * settings button and close button are visible only when the focus is on the buttons (only step 1 and 2 above). Actual focus order [next notification] 1. buttons (if available) 2. Whole notification [previous notification] * settings button and close button are visible when the focus is on the _notifications_ (step 1 and 2 above).
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f16582bfe966e7a06ccd8a37cd89833e4c005212 commit f16582bfe966e7a06ccd8a37cd89833e4c005212 Author: yoshiki iguchi <yoshiki@chromium.org> Date: Fri Jul 14 10:14:35 2017 Fix focus order and button visibility on notification This CL fixes the wrong reverse focus order of notification (see the issue for detail). Previously the close and settings buttons are skipped in reverse order. This also fixes the visibility of close and settings button. These buttons should be visible only when they have a focus. Previously they are visible even when the focus is not on these buttons but on the notification. BUG= 739293 TEST=manual test Change-Id: I8af330314410bf4b1a26be51d86cee312b4344ab Reviewed-on: https://chromium-review.googlesource.com/571522 Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#486719} [modify] https://crrev.com/f16582bfe966e7a06ccd8a37cd89833e4c005212/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/f16582bfe966e7a06ccd8a37cd89833e4c005212/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/f16582bfe966e7a06ccd8a37cd89833e4c005212/ui/message_center/views/notification_control_buttons_view.h [modify] https://crrev.com/f16582bfe966e7a06ccd8a37cd89833e4c005212/ui/message_center/views/notification_view.cc
,
Jul 19 2017
,
Jul 28 2017
The issue on non-ARC notification is finished, but it's still happening on ARC notification.
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/251119cd400fdeaca0a1ec7769a63942e1adac52 commit 251119cd400fdeaca0a1ec7769a63942e1adac52 Author: yoshiki iguchi <yoshiki@chromium.org> Date: Tue Aug 01 06:47:06 2017 Fix the focus order on ARC notification This Cl adds a check of the view of buttons when the focus is moved. The view (ControlButtonsView) is not in the same view hierarchy on ARC++ notifications, so check it separately. So we need to check it separatedly. Bug: 739293 Change-Id: I15695574fd57f4210597bbe38713d75d3faced78 Reviewed-on: https://chromium-review.googlesource.com/588879 Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#490889} [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/arc/notification/arc_notification_content_view_delegate.h [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/arc/notification/arc_notification_view.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/arc/notification/arc_notification_view.h [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/arc/notification/arc_notification_view_unittest.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/message_center_view.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/message_view.h [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/notification_view.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/notification_view.h [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/notification_view_md.cc [modify] https://crrev.com/251119cd400fdeaca0a1ec7769a63942e1adac52/ui/message_center/views/notification_view_md.h
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20b6e16ec5c80b4e34aa8fddd665da99f4fc823f commit 20b6e16ec5c80b4e34aa8fddd665da99f4fc823f Author: yoshiki iguchi <yoshiki@chromium.org> Date: Tue Aug 08 19:09:08 2017 Fix the focus traversal by tab-key on ARC notification The tab-key didn't move the focus on ARC notification. This was a regression of crrev.com/c/582932. This patch fixes it by moving the window delegate to the top level window of ARC notification. Bug: 739293 Change-Id: Ica993313d70d1ba05d20494bcbe81430315e9485 Reviewed-on: https://chromium-review.googlesource.com/604815 Reviewed-by: David Reveman <reveman@chromium.org> Reviewed-by: Peng Huang <penghuang@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#492719} [modify] https://crrev.com/20b6e16ec5c80b4e34aa8fddd665da99f4fc823f/components/exo/notification_surface.cc [modify] https://crrev.com/20b6e16ec5c80b4e34aa8fddd665da99f4fc823f/ui/arc/notification/arc_notification_surface_impl.cc
,
Aug 10 2017
(I deleted the comment 7 since it has wrong info) I'm requesting the merge of the CL mentioned in c#5. The CL in #6 is not necessary since the cause CL is not in M61.
,
Aug 10 2017
,
Aug 10 2017
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 10 2017
Approving merge to M61 Chrome OS.
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63337f3b24c2a6297762d8cea624795e7c898821 commit 63337f3b24c2a6297762d8cea624795e7c898821 Author: yoshiki iguchi <yoshiki@chromium.org> Date: Fri Aug 11 08:56:17 2017 Fix the focus order on ARC notification This Cl adds a check of the view of buttons when the focus is moved. The view (ControlButtonsView) is not in the same view hierarchy on ARC++ notifications, so check it separately. So we need to check it separatedly. TBR=yoshiki@chromium.org (cherry picked from commit 251119cd400fdeaca0a1ec7769a63942e1adac52) Bug: 739293 Change-Id: I15695574fd57f4210597bbe38713d75d3faced78 Reviewed-on: https://chromium-review.googlesource.com/588879 Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#490889} Reviewed-on: https://chromium-review.googlesource.com/610621 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#484} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/arc/notification/arc_notification_content_view_delegate.h [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/arc/notification/arc_notification_view.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/arc/notification/arc_notification_view.h [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/arc/notification/arc_notification_view_unittest.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/message_center_view.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/message_view.h [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/notification_view.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/notification_view.h [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/notification_view_md.cc [modify] https://crrev.com/63337f3b24c2a6297762d8cea624795e7c898821/ui/message_center/views/notification_view_md.h
,
Aug 11 2017
,
Jan 22 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by yoshiki@chromium.org
, Jul 6 2017