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

Issue 2287433003: Get rid of remaining uses of AXObject::elementRect (Closed)

Created:
4 years, 3 months ago by dmazzoni
Modified:
4 years, 3 months ago
Reviewers:
dglazkov, aboxhall
CC:
aboxhall, aboxhall+watch_chromium.org, ajuma+watch-canvas_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-layout_chromium.org, Rik, chromium-reviews, dglazkov+blink, dmazzoni+watch_chromium.org, dmazzoni, dshwang, dtseng+watch_chromium.org, eae+blinkwatch, einbinder+watch-test-runner_chromium.org, haraken, jchaffraix+rendering, je_julie, jochen+watch_chromium.org, Justin Novosad, kinuko+watch, leviw+renderwatch, mlamouri+watch-test-runner_chromium.org, nektar+watch_chromium.org, nektarios, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, yuzo+watch_chromium.org, zoltan1
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Get rid of remaining uses of AXObject::elementRect This change is mostly refactoring and deleting with just a bit of new code. There were a few places we were still using AXObject::elementRect inside of Blink. Fix those and replace them with the new AXObject::getRelativeBounds or a new function AXObject::getBoundsInFrameCoordinates for cases where we do need absolute frame-relative coordates. There were a few subclasses of AXMockObject that didn't support getRelativeBounds yet but weren't migrated earlier because they had no test coverage for their bounding rects. Fix those and add new tests to cover them. To make it easier to implement those, I moved the implementation of getRelativeBounds from AXLayoutObject to AXObject, and made it dependent on a new protected virtual method layoutObjectForRelativeBounds(). If a subclass of AXObject wants its bounds computed all it needs to do is implement layoutObjectForRelativeBounds() and the rest is done for it. BUG=618120 Committed: https://crrev.com/1da67644e2628c06055962acb243c3c058b68c20 Cr-Commit-Position: refs/heads/master@{#417028}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+296 lines, -514 lines) Patch
M components/test_runner/web_ax_object_proxy.cc View 1 1 chunk +4 lines, -2 lines 0 comments Download
A third_party/WebKit/LayoutTests/accessibility/image-map-bounds.html View 1 chunk +35 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/accessibility/slider-thumb-bounds.html View 1 chunk +26 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/accessibility/spin-button-bounds.html View 1 chunk +31 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/line/AbstractInlineTextBox.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/line/AbstractInlineTextBox.cpp View 1 chunk +0 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXImageMapLink.h View 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXImageMapLink.cpp View 3 chunks +11 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp View 1 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h View 2 chunks +2 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp View 8 chunks +3 lines, -256 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXMenuListOption.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXMenuListOption.cpp View 2 chunks +9 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXMenuListPopup.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXNodeObject.h View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp View 1 1 chunk +4 lines, -48 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXObject.h View 1 4 chunks +14 lines, -18 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXObject.cpp View 1 8 chunks +113 lines, -51 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXSlider.h View 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXSlider.cpp View 2 chunks +7 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXSpinButton.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp View 2 chunks +17 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXTableColumn.h View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXTableColumn.cpp View 2 chunks +0 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXTableHeaderContainer.h View 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/accessibility/AXTableHeaderContainer.cpp View 2 chunks +0 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DAPITest.cpp View 2 chunks +10 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/web/WebAXObject.cpp View 1 4 chunks +1 line, -29 lines 0 comments Download
M third_party/WebKit/public/web/WebAXObject.h View 3 chunks +2 lines, -10 lines 0 comments Download

Messages

Total messages: 26 (16 generated)
dmazzoni
Alice, could you take a look first before I loop in owners?
4 years, 3 months ago (2016-08-26 21:23:23 UTC) #3
aboxhall
lgtm https://codereview.chromium.org/2287433003/diff/1/third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp File third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp (right): https://codereview.chromium.org/2287433003/diff/1/third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp#newcode127 third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp:127: // FIXME: This logic should exist in the ...
4 years, 3 months ago (2016-08-26 22:44:46 UTC) #6
dmazzoni
https://codereview.chromium.org/2287433003/diff/1/third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp File third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp (right): https://codereview.chromium.org/2287433003/diff/1/third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp#newcode127 third_party/WebKit/Source/modules/accessibility/AXSpinButton.cpp:127: // FIXME: This logic should exist in the layout ...
4 years, 3 months ago (2016-08-29 23:17:06 UTC) #9
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/2287433003/1
4 years, 3 months ago (2016-08-29 23:59:17 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/248704)
4 years, 3 months ago (2016-08-30 03:41:27 UTC) #13
dmazzoni
+dglazkov for public/web/
4 years, 3 months ago (2016-08-30 06:13:53 UTC) #15
dglazkov
lgtm
4 years, 3 months ago (2016-09-06 23:33:25 UTC) #16
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/2287433003/20001
4 years, 3 months ago (2016-09-07 19:27:26 UTC) #23
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 3 months ago (2016-09-07 19:34:43 UTC) #24
commit-bot: I haz the power
4 years, 3 months ago (2016-09-07 19:38:07 UTC) #26
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/1da67644e2628c06055962acb243c3c058b68c20
Cr-Commit-Position: refs/heads/master@{#417028}

Powered by Google App Engine
This is Rietveld 408576698