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

Issue 2372253003: Account for device scale factor in automation API bounding boxes. (Closed)

Created:
4 years, 2 months ago by dmazzoni
Modified:
4 years, 2 months ago
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Account for device scale factor in automation API bounding boxes. Bounding boxes from web pages were including the device scale factor and all other bounding boxes were not. Fix this in the automation API so that everything ChromeVox deals with is consistently "CSS pixels", i.e. without the device scale factor applied. Since we want to apply this factor for some AX trees but not others, I took this opportunity to consolidate a few "magic constants" and have a single shared constant for the desktop tree across the automation API. BUG=638701 Committed: https://crrev.com/855770d4a1e3c30a3d08f512043b9c04966922e2 Cr-Commit-Position: refs/heads/master@{#421937}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -4 lines) Patch
M chrome/browser/extensions/api/automation_internal/automation_event_router.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/automation_internal/automation_internal_api.cc View 4 chunks +3 lines, -3 lines 0 comments Download
A chrome/common/extensions/api/automation_api_constants.h View 1 chunk +20 lines, -0 lines 0 comments Download
M chrome/renderer/extensions/automation_internal_custom_bindings.cc View 2 chunks +12 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 10 (3 generated)
dmazzoni
4 years, 2 months ago (2016-09-27 17:36:15 UTC) #2
aboxhall
lgtm - is this testable? Definitely not going to block on that, just curious.
4 years, 2 months ago (2016-09-27 18:43:59 UTC) #3
dmazzoni
The reason it's nontrivial to test is that all of the automation tests are API ...
4 years, 2 months ago (2016-09-27 18:54:08 UTC) #4
Ken Rockot(use gerrit already)
lgtm
4 years, 2 months ago (2016-09-27 18:57:41 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2372253003/1
4 years, 2 months ago (2016-09-29 20:42:16 UTC) #7
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 2 months ago (2016-09-29 21:21:37 UTC) #8
commit-bot: I haz the power
4 years, 2 months ago (2016-09-29 21:23:18 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/855770d4a1e3c30a3d08f512043b9c04966922e2
Cr-Commit-Position: refs/heads/master@{#421937}

Powered by Google App Engine
This is Rietveld 408576698