Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(43)

Issue 23506027: Implement web content hit testing in win aura. (Closed)

Created:
7 years, 3 months ago by dmazzoni
Modified:
7 years, 3 months ago
Reviewers:
Avi (use Gerrit), sky, Jói
CC:
chromium-reviews, jbauman+watch_chromium.org, James Su, sievers+watch_chromium.org, hashimoto+watch_chromium.org, tfarina, aboxhall+watch_chromium.org, jam, yoshiki+watch_chromium.org, penghuang+watch_chromium.org, yuzo+watch_chromium.org, davidbarr+watch_chromium.org, joi+watch-content_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, yusukes+watch_chromium.org, piman+watch_chromium.org, danakj+watch_chromium.org, ctguil+watch_chromium.org, miu+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Implement web content hit testing in win aura. NativeViewAccessibilityWin needs to do hit testing in web content as well as views. While implementing this, I realized that it isn't necessary to have a special interface to a RWHV to get an object by child id - we can just rely on WebView overriding GetNativeViewAccessible and do the lookup using a native call. The same trick works for hit testing - we check if the point is inside a child's bounding rect, and if so we call accHitTest on that child's native view accessible - and since WebView overrides GetNativeViewAccessible, the recursive call to accHitTest will return the correct result from inside the web content. We still need AccessibleWebViewRegistry (because looking up objects from child ids gets called A LOT, but it's much simpler now, just a set of Views. BUG=284803 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221744

Patch Set 1 #

Patch Set 2 : Better logic #

Total comments: 10

Patch Set 3 : Address feedback #

Patch Set 4 : Fix compile error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -113 lines) Patch
M content/browser/renderer_host/render_widget_host_view_aura.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura.cc View 1 2 3 1 chunk +0 lines, -12 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_guest.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_guest.cc View 1 2 3 1 chunk +0 lines, -8 lines 0 comments Download
M content/browser/renderer_host/test_render_view_host.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/renderer_host/test_render_view_host.cc View 1 chunk +0 lines, -8 lines 0 comments Download
M content/public/browser/render_widget_host_view.h View 1 chunk +0 lines, -5 lines 0 comments Download
M ui/views/accessibility/native_view_accessibility.h View 1 2 2 chunks +5 lines, -10 lines 0 comments Download
M ui/views/accessibility/native_view_accessibility.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M ui/views/accessibility/native_view_accessibility_win.cc View 1 2 10 chunks +78 lines, -27 lines 0 comments Download
M ui/views/controls/webview/webview.h View 2 chunks +1 line, -7 lines 0 comments Download
M ui/views/controls/webview/webview.cc View 2 chunks +1 line, -22 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
dmazzoni
7 years, 3 months ago (2013-09-04 06:34:21 UTC) #1
sky
I like less code:) https://codereview.chromium.org/23506027/diff/3001/ui/views/accessibility/native_view_accessibility.h File ui/views/accessibility/native_view_accessibility.h (right): https://codereview.chromium.org/23506027/diff/3001/ui/views/accessibility/native_view_accessibility.h#newcode29 ui/views/accessibility/native_view_accessibility.h:29: static void RegisterWebView(View* web_view); Should ...
7 years, 3 months ago (2013-09-04 16:07:32 UTC) #2
dmazzoni
+avi for content/public/browser https://codereview.chromium.org/23506027/diff/3001/ui/views/accessibility/native_view_accessibility.h File ui/views/accessibility/native_view_accessibility.h (right): https://codereview.chromium.org/23506027/diff/3001/ui/views/accessibility/native_view_accessibility.h#newcode29 ui/views/accessibility/native_view_accessibility.h:29: static void RegisterWebView(View* web_view); On 2013/09/04 ...
7 years, 3 months ago (2013-09-04 17:32:08 UTC) #3
sky
LGTM
7 years, 3 months ago (2013-09-04 20:12:17 UTC) #4
sky
-avi@google +avi@chromium for content/public/browser
7 years, 3 months ago (2013-09-04 20:13:07 UTC) #5
dmazzoni
+joi for content/public/browser
7 years, 3 months ago (2013-09-06 07:44:00 UTC) #6
Jói
//content/public LGTM.
7 years, 3 months ago (2013-09-06 08:26:53 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/23506027/14001
7 years, 3 months ago (2013-09-06 14:00:50 UTC) #8
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 3 months ago (2013-09-06 14:35:09 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/23506027/40001
7 years, 3 months ago (2013-09-06 14:48:56 UTC) #10
commit-bot: I haz the power
7 years, 3 months ago (2013-09-06 19:03:20 UTC) #11
Message was sent while issue was closed.
Change committed as 221744

Powered by Google App Engine
This is Rietveld 408576698