Issue 1128183010: ATK accessibility support for web content on Aura Linux.

Issue 1128183010: ATK accessibility support for web content on Aura Linux. (Closed)

Created:
5 years, 7 months ago by shreeramk

Modified:
5 years, 5 months ago

Reviewers:
Avi (use Gerrit), je_julie(Not used), jochen (gone - plz use gerrit), aboxhall, dmazzoni, Peter Lundblad

CC:
aboxhall+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, jam, je_julie, nektar+watch_chromium.org, plundblad+watch_chromium.org, yuzo+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

ATK accessibility support for web content on Aura Linux. This CL adds Accessibility support to Chromium browser for linux platform. It successfully exposes an AX Tree for content. You can test this with chrome://accessibility. This is just the initial CL to get the accessibility support on Linux. Other interfaces need to be implemented to make it useful for accessibility tools. ATK_COMPONENT & ATK_VALUE Interfaces has been implemented in this CL. Content: <html> <body> <input type="checkbox"> </body> </html> AX Tree: [document web] enabled focusable focused id=1 ++[section] enabled id=4 ++++[check box] enabled focusable id=5 BUG=463671 Committed: https://crrev.com/792fba03a8605accd9016cc0a6fb4a7ce52dedc6 Cr-Commit-Position: refs/heads/master@{#339435}

Patch Set 1 #

Patch Set 2 : Fixing nits #

Total comments: 6

Patch Set 3 : Incorporating Comments #

Patch Set 4 : fixing content_unittests #

Total comments: 1

Patch Set 5 : Fixing chromeos bot failures #

Patch Set 6 #

Patch Set 7 : rebasing #

Patch Set 8 : #

Patch Set 9 : #

Patch Set 10 : #

Total comments: 1

Patch Set 11 : adding changes again in BUILD.gn #

Patch Set 12 : #

Total comments: 2

Patch Set 13 : #

Total comments: 1

Patch Set 14 : fixing nits #

Created: 5 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1040 lines, -37 lines)			Patch
M	build/config/linux/BUILD.gn	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+26 lines, -0 lines	0 comments	Download
M	build/config/ui.gni	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+3 lines, -0 lines	0 comments	Download
M	content/browser/BUILD.gn	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+14 lines, -0 lines	0 comments	Download
M	content/browser/accessibility/accessibility_tree_formatter.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+4 lines, -1 line	0 comments	Download
A	content/browser/accessibility/accessibility_tree_formatter_auralinux.cc	View		1 chunk	+106 lines, -0 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility.h	View	1 2 3 4 5 6 7 8 9 10 11 12	2 chunks	+4 lines, -0 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+6 lines, -5 lines	0 comments	Download
A	content/browser/accessibility/browser_accessibility_auralinux.h	View		1 chunk	+92 lines, -0 lines	0 comments	Download
A	content/browser/accessibility/browser_accessibility_auralinux.cc	View	1 2 3	1 chunk	+672 lines, -0 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager.h	View	1 2 3 4 5 6 7 8 9 10 11 12 13	2 chunks	+8 lines, -2 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager.cc	View	1 2 3 4 5 6 7 8 9 10 11 12	1 chunk	+6 lines, -5 lines	0 comments	Download
A +	content/browser/accessibility/browser_accessibility_manager_auralinux.h	View	1 2 3 4 5 6	1 chunk	+19 lines, -23 lines	0 comments	Download
A	content/browser/accessibility/browser_accessibility_manager_auralinux.cc	View	1 2 3 4	1 chunk	+56 lines, -0 lines	0 comments	Download
M	content/content_browser.gypi	View	1 2 3 4 5 6 7 8 9 10 11 12 13	2 chunks	+23 lines, -0 lines	0 comments	Download
M	ui/accessibility/BUILD.gn	View	1 2 3 4 5 6 7 8 9 10 11 12 13	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 41 (7 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

shreeramk

@Dominic & @aboxhall: This is initial patch to support Accessibility on Linux platform. Please provide ...

5 years, 7 months ago (2015-05-14 11:35:31 UTC) #2

shreeramk

Currently role is added using atk_role_get_name but I was thinking of adding an util file ...

5 years, 7 months ago (2015-05-14 13:50:48 UTC) #3

shreeramk

On 2015/05/14 13:50:48, shreeramk wrote: > Currently role is added using atk_role_get_name but I was ...

5 years, 7 months ago (2015-05-18 18:42:26 UTC) #4

dmazzoni

I've started taking a look, I'll get back to you shortly. On Mon, May 18, ...

5 years, 7 months ago (2015-05-18 18:52:09 UTC) #5

dmazzoni

Nice job! So, my original hope was that we would eventually make all of the ...

5 years, 7 months ago (2015-05-22 22:25:32 UTC) #6

shreeramk

Bots related to *_gn_* and chromeos are failing. Are the changes in BUILD.gn and content_browser.gypi ...

5 years, 7 months ago (2015-05-25 04:33:50 UTC) #7

dmazzoni

On 2015/05/25 04:33:50, shreeramk wrote: > Bots related to *_gn_* and chromeos are failing. Are ...

5 years, 7 months ago (2015-05-25 06:05:19 UTC) #8

shreeramk

I think this failed test cases were written only for mac. content_unittests (with patch) content_unittests ...

5 years, 7 months ago (2015-05-26 05:59:52 UTC) #9

dmazzoni

On Mon, May 25, 2015 at 10:59 PM, <shreeram.k@samsung.com> wrote: > I think this failed ...

5 years, 7 months ago (2015-05-26 18:46:50 UTC) #10

shreeramk

On 2015/05/26 18:46:50, dmazzoni wrote: > On Mon, May 25, 2015 at 10:59 PM, <mailto:shreeram.k@samsung.com> ...

5 years, 7 months ago (2015-05-26 19:41:25 UTC) #11

shreeramk

Now content unittests is passing for linux as well. But still chromeos and *_gn_* bots ...

5 years, 7 months ago (2015-05-27 16:52:10 UTC) #12

dmazzoni

Yeah, content_browser.gypi needs to exclude the _auralinux source files if it's not the right platform. ...

5 years, 7 months ago (2015-05-27 16:58:00 UTC) #13

shreeramk

https://codereview.chromium.org/1128183010/diff/60001/content/browser/accessibility/browser_accessibility_manager_auralinux.cc File content/browser/accessibility/browser_accessibility_manager_auralinux.cc (right): https://codereview.chromium.org/1128183010/diff/60001/content/browser/accessibility/browser_accessibility_manager_auralinux.cc#newcode3 content/browser/accessibility/browser_accessibility_manager_auralinux.cc:3: // found in the LICENSE file. Some of *_rel_ng ...

5 years, 7 months ago (2015-05-27 16:58:01 UTC) #14

dmazzoni

On 2015/06/01 17:32:13, shreeramk wrote: > @Dominic - Any input on this bot failures? :( ...

5 years, 6 months ago (2015-06-02 05:31:44 UTC) #16

dmazzoni

On 2015/06/01 17:32:13, shreeramk wrote: > @Dominic - Any input on this bot failures? :( ...

5 years, 6 months ago (2015-06-02 05:31:48 UTC) #17

shreeramk

avi@chromium.org: I am facing this issue related to bot failures on three bots 1. linux_chromium_gn_chromeos_rel ...

5 years, 5 months ago (2015-07-02 06:51:52 UTC) #20

Avi (use Gerrit)

The linker is complaining that it can't find the definitions of your functions. Given that ...

5 years, 5 months ago (2015-07-02 18:25:27 UTC) #21

shreeramk

On 2015/07/02 18:25:27, Avi wrote: > The linker is complaining that it can't find the ...

5 years, 5 months ago (2015-07-03 02:59:06 UTC) #22

Avi (use Gerrit)

OK, patch set 9. cast_shell_linux fails: function content::RenderWidgetHostViewChildFrame::CreateBrowserAccessibilityManager(): function content::BrowserAccessibilityManagerTest_TestNoLeaks_Test::TestBody(): function content::BrowserAccessibilityManagerTest_TestNoLeaks_Test::TestBody(): function content::BrowserAccessibilityManagerTest_TestReuseBrowserAccessibilityObjects_Test::TestBody(): function ...

5 years, 5 months ago (2015-07-03 15:22:52 UTC) #23

OK, patch set 9. cast_shell_linux fails:

function
content::RenderWidgetHostViewChildFrame::CreateBrowserAccessibilityManager():
function content::BrowserAccessibilityManagerTest_TestNoLeaks_Test::TestBody():
function content::BrowserAccessibilityManagerTest_TestNoLeaks_Test::TestBody():
function
content::BrowserAccessibilityManagerTest_TestReuseBrowserAccessibilityObjects_Test::TestBody():
function
content::BrowserAccessibilityManagerTest_TestReuseBrowserAccessibilityObjects2_Test::TestBody():
  undefined reference to 'content::BrowserAccessibilityManager::Create()'

function content::BrowserAccessibilityFactory::Create():
  undefined reference to 'content::BrowserAccessibility::Create()'

function content::AccessibilityTreeFormatter::Create():
  undefined reference to 'content::AccessibilityTreeFormatter::Initialize()'

function content::AccessibilityTreeFormatter::RecursiveBuildAccessibilityTree():
  undefined reference to 'content::AccessibilityTreeFormatter::AddProperties()'

function
content::AccessibilityTreeFormatter::RecursiveFormatAccessibilityTree():
  undefined reference to 'content::AccessibilityTreeFormatter::ToString()'

So those are the five functions that the linker can't find. Let's look at one of
them, content::BrowserAccessibilityManager::Create().

https://codereview.chromium.org/1128183010/diff/160001/content/browser/access...
has:

OLD:
#if !defined(OS_MACOSX) && \
    !defined(OS_WIN) && \
    !defined(OS_ANDROID)

NEW:
#if defined(OS_CHROMEOS)

So what happens is that cast_shell_linux used to use that file's definition of
BrowserAccessibilityManager::Create(), because it is neither Mac, Windows, or
Android. It can't use that any more, though, because (apparently) it's not
ChromeOS. But there doesn't seem to be _any_ definition now for it.

You also comment:
// We have subclassess of BrowserAccessibility on all platforms except ChromeOS.
// For ChromeOS platform, instantiate the base class.

This first line appears to not be true. What version of the BrowserAccessibility
class does the cast shell use? The linker is telling us that we don't have one.

shreeramk

> So those are the five functions that the linker can't find. Let's look at ...

5 years, 5 months ago (2015-07-04 17:08:45 UTC) #24

Avi (use Gerrit)

Two replies, one to each issue. > So, > cast_shell_linux should use Classes and methods ...

5 years, 5 months ago (2015-07-04 18:03:53 UTC) #25

shreeramk

> As an aside, I'm being a bit vague because I have no idea what's ...

5 years, 5 months ago (2015-07-04 19:12:11 UTC) #26

shreeramk

@Dominic: I have made some changes in content_browser.gypi file. Please take a look. https://codereview.chromium.org/1128183010/diff/220001/content/content_browser.gypi File ...

5 years, 5 months ago (2015-07-14 05:21:00 UTC) #27

Peter Lundblad

Hi shreeram, I took a look at this and think I've fixed all compilation issues ...

5 years, 5 months ago (2015-07-15 14:48:34 UTC) #30

shreeramk

@Peter: Thanks a Lot. I took the changes done by you and now I can ...

5 years, 5 months ago (2015-07-16 10:37:43 UTC) #31

dmazzoni

lgtm https://codereview.chromium.org/1128183010/diff/240001/content/browser/accessibility/browser_accessibility_manager.h File content/browser/accessibility/browser_accessibility_manager.h (right): https://codereview.chromium.org/1128183010/diff/240001/content/browser/accessibility/browser_accessibility_manager.h#newcode235 content/browser/accessibility/browser_accessibility_manager.h:235: ToBrowserAccessibilityManagerAuraLinux(); nit: indent by 2 more

5 years, 5 months ago (2015-07-17 15:56:49 UTC) #33

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1128183010/260001

5 years, 5 months ago (2015-07-20 11:04:40 UTC) #39

commit-bot: I haz the power

5 years, 5 months ago (2015-07-20 11:44:54 UTC) #41

Message was sent while issue was closed.

Patchset 14 (id:??) landed as
https://crrev.com/792fba03a8605accd9016cc0a6fb4a7ce52dedc6
Cr-Commit-Position: refs/heads/master@{#339435}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages