New issue
Advanced search Search tips

Issue 877487 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 25
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug
Team-Accessibility



Sign in to add a comment

Redundant MSAA location changed events for caret

Project Member Reported by aleventhal@chromium.org, Aug 24

Issue description

Currently when the caret moves in content, we fire two MSAA location change events for the caret:
1. In the Chrome_RenderWidgetHostHWND (content window)
2. Chrome_WidgetWin_1 (UI window)

ZoomText is the primary consumer of these events, and they've asked that we only fire them for the Chrome_WidgetWin_1. This keeps their logic simpler and is more performant from their point of view.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 27

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

commit 62c87f0fab8b988fdc1fe39cf8e0c54bdbd12815
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Mon Aug 27 20:45:00 2018

Remove redundant caret events

Previously, when the caret moves in content, we fire two MSAA location
change events for the caret:
1. In the Chrome_RenderWidgetHostHWND (content window)
2. Chrome_WidgetWin_1 (UI window)

The ZoomText screen magnifier application is the primary consumer of
these events, and they've asked that we only fire them for the
Chrome_WidgetWin_1. This keeps their logic simpler and is more
performant from their point of view.

TBR=sadrul@chromium.org
NOTRY=true

Bug:  877487 
Change-Id: I05a2d1ba1d6c13c5a5aa4a16c9b91826301eb1f9
Reviewed-on: https://chromium-review.googlesource.com/1188603
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586406}
[modify] https://crrev.com/62c87f0fab8b988fdc1fe39cf8e0c54bdbd12815/content/browser/renderer_host/legacy_render_widget_host_win.cc
[modify] https://crrev.com/62c87f0fab8b988fdc1fe39cf8e0c54bdbd12815/content/browser/renderer_host/legacy_render_widget_host_win.h
[modify] https://crrev.com/62c87f0fab8b988fdc1fe39cf8e0c54bdbd12815/content/browser/renderer_host/render_widget_host_view_aura.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 24

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

commit 12fb3159938f0aa4153b646298a7248e178df8c3
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Mon Sep 24 14:37:14 2018

Only fire caret location change if it actually changes

Also add tests for caret events.

Bug:  877487 
Change-Id: Ib9ac49e60c218169e8108ef83b000fcaaf176570
Reviewed-on: https://chromium-review.googlesource.com/1189184
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593533}
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/browser/accessibility/accessibility_event_recorder_win.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/browser/accessibility/dump_accessibility_browsertest_base.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/browser/accessibility/dump_accessibility_browsertest_base.h
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/browser/accessibility/dump_accessibility_events_browsertest.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/test/data/accessibility/event/aria-combo-box-focus.html
[add] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/test/data/accessibility/event/caret-move-expected-win.txt
[add] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/test/data/accessibility/event/caret-move.html
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/content/test/data/accessibility/readme.md
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/ui/accessibility/ax_node_data.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/ui/accessibility/ax_node_data.h
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/ui/accessibility/platform/ax_system_caret_win.cc
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/ui/accessibility/platform/ax_system_caret_win.h
[modify] https://crrev.com/12fb3159938f0aa4153b646298a7248e178df8c3/ui/views/win/hwnd_message_handler.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 25

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

commit 970fa0939bbfb466ddd4f5bc5c54723a96756213
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Sep 25 10:14:34 2018

Revert "Only fire caret location change if it actually changes"

This reverts commit 12fb3159938f0aa4153b646298a7248e178df8c3.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 593533 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMTJmYjMxNTk5MzhmMGFhNDE1M2I2NDYyOThhNzI0OGUxNzhkZjhjMww

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.win/Win10%20Tests%20x64/27833

Sample Failed Step: network_service_content_browsertests on Windows-10-15063

Sample Flaky Test: DumpAccessibilityEventsTest.AccessibilityEventsCaretMove

Original change's description:
> Only fire caret location change if it actually changes
>
> Also add tests for caret events.
>
> Bug:  877487 
> Change-Id: Ib9ac49e60c218169e8108ef83b000fcaaf176570
> Reviewed-on: https://chromium-review.googlesource.com/1189184
> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593533}

No-Presubmit: true
Bug:  877487 ,  888877 
Change-Id: Ic6fe9dc3c454c8a5602e0ac12671e7061f9939fe
Reviewed-on: https://chromium-review.googlesource.com/1242903
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593883}
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/browser/accessibility/accessibility_event_recorder_win.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/browser/accessibility/dump_accessibility_browsertest_base.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/browser/accessibility/dump_accessibility_browsertest_base.h
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/browser/accessibility/dump_accessibility_events_browsertest.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/test/data/accessibility/event/aria-combo-box-focus.html
[delete] https://crrev.com/dd97891ac088be34a2d7f60e068a15d7a0ff2435/content/test/data/accessibility/event/caret-move-expected-win.txt
[delete] https://crrev.com/dd97891ac088be34a2d7f60e068a15d7a0ff2435/content/test/data/accessibility/event/caret-move.html
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/content/test/data/accessibility/readme.md
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/ui/accessibility/ax_node_data.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/ui/accessibility/ax_node_data.h
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/ui/accessibility/platform/ax_system_caret_win.cc
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/ui/accessibility/platform/ax_system_caret_win.h
[modify] https://crrev.com/970fa0939bbfb466ddd4f5bc5c54723a96756213/ui/views/win/hwnd_message_handler.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 25

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

commit aaa5431ae63ba2f2b480124b38c201f1b55886f1
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Tue Sep 25 15:55:52 2018

Undo revert "Only fire caret location change if it actually changes"

Disabling flaky tests for now.

TBR=dmazzoni,sadrul

Bug:  877487 
Change-Id: Idd03f723315b725fc8dab7222577f33af06b156e
Reviewed-on: https://chromium-review.googlesource.com/1243223
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593953}
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/browser/accessibility/accessibility_event_recorder_win.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/browser/accessibility/dump_accessibility_browsertest_base.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/browser/accessibility/dump_accessibility_browsertest_base.h
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/browser/accessibility/dump_accessibility_events_browsertest.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/test/data/accessibility/event/aria-combo-box-focus.html
[add] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/test/data/accessibility/event/caret-move-expected-win.txt
[add] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/test/data/accessibility/event/caret-move.html
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/content/test/data/accessibility/readme.md
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/ui/accessibility/ax_node_data.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/ui/accessibility/ax_node_data.h
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/ui/accessibility/platform/ax_system_caret_win.cc
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/ui/accessibility/platform/ax_system_caret_win.h
[modify] https://crrev.com/aaa5431ae63ba2f2b480124b38c201f1b55886f1/ui/views/win/hwnd_message_handler.cc

Status: Fixed (was: Started)

Sign in to add a comment