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

Issue 1039873002: AX presentation role should be inherited to its required owned elements. (Closed)

Created:
5 years, 9 months ago by je_julie(Not used)
Modified:
5 years, 8 months ago
Reviewers:
dmazzoni
CC:
blink-reviews, nektarios, dmazzoni, je_julie(Not used), aboxhall
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

AX presentation role should be inherited to its required owned elements. This patch makes AX presentation role be inherited to its required owned elements. The variable, m_cachedHasInheritedPresentationRole, is added and used to test whether AXObject has inheritance for presentation or not. It’s recalculated on computeHasInheritedPresentationRole(). In testing presentation inheritance, several conditions are needed. It should not be focusable, not have an explicit role definition and be a required owned element. BUG=471626 R=dmazzoni Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193042

Patch Set 1 #

Total comments: 6

Patch Set 2 : Moved code to AXNodeObject #

Total comments: 20

Patch Set 3 : Change member functions to static local functions #

Total comments: 9

Patch Set 4 : Rename #

Total comments: 4

Patch Set 5 : Update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+310 lines, -35 lines) Patch
M LayoutTests/accessibility/listitem-presentation-inherited-expected.txt View 1 chunk +0 lines, -3 lines 0 comments Download
A LayoutTests/accessibility/presentation-owned-elements.html View 1 2 3 4 1 chunk +82 lines, -0 lines 0 comments Download
A LayoutTests/accessibility/presentation-owned-elements-expected.txt View 1 2 3 4 1 chunk +106 lines, -0 lines 0 comments Download
M Source/modules/accessibility/AXLayoutObject.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M Source/modules/accessibility/AXLayoutObject.cpp View 1 2 3 2 chunks +1 line, -31 lines 0 comments Download
M Source/modules/accessibility/AXListBoxOption.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/accessibility/AXListBoxOption.cpp View 1 2 3 4 1 chunk +24 lines, -0 lines 0 comments Download
M Source/modules/accessibility/AXNodeObject.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/accessibility/AXNodeObject.cpp View 1 2 3 2 chunks +88 lines, -0 lines 0 comments Download
M Source/modules/accessibility/AXObject.h View 1 2 3 4 chunks +5 lines, -0 lines 0 comments Download
M Source/modules/accessibility/AXObject.cpp View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (4 generated)
je_julie(Not used)
Hi Dominic, This patch makes presentation role be inherited to its required owned elements. There ...
5 years, 9 months ago (2015-03-28 10:54:36 UTC) #2
dmazzoni
Great fix. Please file a bug and add a bug number. It's nice to have ...
5 years, 8 months ago (2015-03-29 07:46:22 UTC) #3
je_julie(Not used)
Hi Dominic, I filed a new bug and added bug number to description. With the ...
5 years, 8 months ago (2015-03-31 16:14:28 UTC) #4
dmazzoni
https://codereview.chromium.org/1039873002/diff/20001/Source/modules/accessibility/AXLayoutObject.cpp File Source/modules/accessibility/AXLayoutObject.cpp (right): https://codereview.chromium.org/1039873002/diff/20001/Source/modules/accessibility/AXLayoutObject.cpp#newcode566 Source/modules/accessibility/AXLayoutObject.cpp:566: if (isPresentationRole(static_cast<const AXObject*>(this))) Can you do this without the ...
5 years, 8 months ago (2015-03-31 16:35:24 UTC) #5
je_julie(Not used)
I updated code as you commented. PTAL. https://codereview.chromium.org/1039873002/diff/20001/Source/modules/accessibility/AXLayoutObject.cpp File Source/modules/accessibility/AXLayoutObject.cpp (right): https://codereview.chromium.org/1039873002/diff/20001/Source/modules/accessibility/AXLayoutObject.cpp#newcode566 Source/modules/accessibility/AXLayoutObject.cpp:566: if (isPresentationRole(static_cast<const ...
5 years, 8 months ago (2015-04-01 12:51:28 UTC) #7
dmazzoni
https://codereview.chromium.org/1039873002/diff/60001/Source/modules/accessibility/AXListBoxOption.cpp File Source/modules/accessibility/AXListBoxOption.cpp (right): https://codereview.chromium.org/1039873002/diff/60001/Source/modules/accessibility/AXListBoxOption.cpp#newcode85 Source/modules/accessibility/AXListBoxOption.cpp:85: if (layoutObject->isListBox() && (parent->isPresentation() || parent->hasInheritedPresentationRole())) Do you need ...
5 years, 8 months ago (2015-04-01 15:27:55 UTC) #8
je_julie(Not used)
Thanks for your comment. I updated patchset. PTAL. https://codereview.chromium.org/1039873002/diff/60001/Source/modules/accessibility/AXListBoxOption.cpp File Source/modules/accessibility/AXListBoxOption.cpp (right): https://codereview.chromium.org/1039873002/diff/60001/Source/modules/accessibility/AXListBoxOption.cpp#newcode85 Source/modules/accessibility/AXListBoxOption.cpp:85: if ...
5 years, 8 months ago (2015-04-02 05:51:28 UTC) #10
dmazzoni
Thanks for your work on this change. It ended up being tricky but it's worth ...
5 years, 8 months ago (2015-04-02 06:22:18 UTC) #11
je_julie(Not used)
Hi Dominic, I think that code is getting clear through the review and I appreciate ...
5 years, 8 months ago (2015-04-02 13:38:08 UTC) #12
dmazzoni
lgtm
5 years, 8 months ago (2015-04-02 13:53:20 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1039873002/120001
5 years, 8 months ago (2015-04-02 13:53:32 UTC) #15
commit-bot: I haz the power
5 years, 8 months ago (2015-04-02 15:07:09 UTC) #16
Message was sent while issue was closed.
Committed patchset #5 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193042

Powered by Google App Engine
This is Rietveld 408576698