Expose "Chrome" as accessible product name.

Expose "Chrome" as accessible product name.

Moves source of the various components of the user agent
product name and version to ChromeVersionInfo, and
exposes them in ChromeContentClient.

BUG=255053
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209317

[jam, 2013-06-28 16:15:30]

i'm curious, why did this stop working? a change in chrome or a change in jaws?

[dmazzoni, 2013-06-28 16:32:52]

On 2013/06/28 16:15:30, jam wrote:
> i'm curious, why did this stop working? a change in chrome or a change in
jaws?

Both at once. They were detecting Chrome using the window class name, and I
asked them to stop because we wanted to be able to change that. We agreed to use
IAccessibleApplication to expose the application name instead. They pushed a
change to switch to using IAccessibleApplication, and I pushed a change to
expose it - but they were expecting "Chrome" as the product name and I was
exporting "Chrome/a.b.c.d".

I didn't catch it right away because it didn't completely break. Basic stuff
still worked, but some custom browser behavior (like the custom Find dialog)
stopped working.

I've asked them to enable their Chrome mode when the toolkitName is Chrome so
that Opera and Chrome can share the same accessibility interface.

[jam, 2013-06-28 19:04:40]

On 2013/06/28 16:32:52, Dominic Mazzoni wrote:
> On 2013/06/28 16:15:30, jam wrote:
> > i'm curious, why did this stop working? a change in chrome or a change in
> jaws?
> 
> Both at once. They were detecting Chrome using the window class name, and I
> asked them to stop because we wanted to be able to change that. We agreed to
use
> IAccessibleApplication to expose the application name instead. They pushed a
> change to switch to using IAccessibleApplication, and I pushed a change to
> expose it - but they were expecting "Chrome" as the product name and I was
> exporting "Chrome/a.b.c.d".
> 
> I didn't catch it right away because it didn't completely break. Basic stuff
> still worked, but some custom browser behavior (like the custom Find dialog)
> stopped working.
> 
> I've asked them to enable their Chrome mode when the toolkitName is Chrome so
> that Opera and Chrome can share the same accessibility interface.

I see. so, since you want this accessibility code to work the same for all
content-based browsers, why do we need this change? i.e. why not just hardcode
browser_accessibility_win.cc to return "Chrome"?

[dmazzoni, 2013-06-28 21:41:07]

On 2013/06/28 19:04:40, jam wrote:
> I see. so, since you want this accessibility code to work the same for all
> content-based browsers, why do we need this change? i.e. why not just hardcode
> browser_accessibility_win.cc to return "Chrome"?

IAccessibleApplication returns four fields: "App" and "Toolkit", plus the
version number for each. I'd like Chrome and Chromium to return "Chrome" and
"Chrome", respectively.

I'd like Opera to return "Opera" and "Chrome", respectively - so that's why I
want the app string to come from embedder code.

I think this is a reasonable use of the toolkit field - a Qt-based app would
return "Qt" for toolkit, allowing screen readers to handle any Qt-specific
customization generically. Similarly, any browser that uses Chrome's content
engine.

Changing the app name to "Chrome" will fix JAWS now - but ideally they'll update
to look at the toolkit name in the future, so then Opera will start working as
well.

[jam, 2013-06-28 22:16:13]

On 2013/06/28 21:41:07, Dominic Mazzoni wrote:
> On 2013/06/28 19:04:40, jam wrote:
> > I see. so, since you want this accessibility code to work the same for all
> > content-based browsers, why do we need this change? i.e. why not just
hardcode
> > browser_accessibility_win.cc to return "Chrome"?
> 
> IAccessibleApplication returns four fields: "App" and "Toolkit", plus the
> version number for each. I'd like Chrome and Chromium to return "Chrome" and
> "Chrome", respectively.
> 
> I'd like Opera to return "Opera" and "Chrome", respectively - so that's why I
> want the app string to come from embedder code.
> 
> I think this is a reasonable use of the toolkit field - a Qt-based app would
> return "Qt" for toolkit, allowing screen readers to handle any Qt-specific
> customization generically. Similarly, any browser that uses Chrome's content
> engine.
> 
> Changing the app name to "Chrome" will fix JAWS now - but ideally they'll
update
> to look at the toolkit name in the future, so then Opera will start working as
> well.

I'm not very familiar with accessibility, so excuse the naive questions :)

So in Opera does it have its own accessibility code elsewhere that handles its
UI?

In the interest of keeping the content api small. Can we just say that
GetProduct returns a string in the format of "app name/version", and then this
accessibility code can parse it?

[dmazzoni, 2013-06-28 22:24:17]

On Fri, Jun 28, 2013 at 3:16 PM, <jam@chromium.org> wrote:

> I'm not very familiar with accessibility, so excuse the naive questions :)
>
> So in Opera does it have its own accessibility code elsewhere that handles
> its
> UI?
>

Anything outside of web content is up to them. The location bar and
toolbars and stuff really don't take too much work, and I believe their
stuff already works.


> In the interest of keeping the content api small. Can we just say that
> GetProduct returns a string in the format of "app name/version", and then
> this
> accessibility code can parse it?
>

I guess that's fine. I was a bit afraid of depending on that and then
having it break after some future refactoring.

What do you think of the change to move the implementation of the product
string to VersionInfo, though? BrowserCommands calling into
ChromeContentClient doesn't seem right.

- Dominic

[jam, 2013-06-28 22:37:43]

On 2013/06/28 22:24:17, Dominic Mazzoni wrote:
> On Fri, Jun 28, 2013 at 3:16 PM, <mailto:jam@chromium.org> wrote:
> 
> > I'm not very familiar with accessibility, so excuse the naive questions :)
> >
> > So in Opera does it have its own accessibility code elsewhere that handles
> > its
> > UI?
> >
> 
> Anything outside of web content is up to them. The location bar and
> toolbars and stuff really don't take too much work, and I believe their
> stuff already works.
> 
> 
> > In the interest of keeping the content api small. Can we just say that
> > GetProduct returns a string in the format of "app name/version", and then
> > this
> > accessibility code can parse it?
> >
> 
> I guess that's fine. I was a bit afraid of depending on that and then
> having it break after some future refactoring.

yeah, but we can keep that until it needs to change. i just really like to keep
ContentClient minimal.

> 
> What do you think of the change to move the implementation of the product
> string to VersionInfo, though? BrowserCommands calling into
> ChromeContentClient doesn't seem right.

that looks good, thanks
> 
> - Dominic

[dmazzoni, 2013-06-28 22:56:37]

Done, take a look.

[jam, 2013-06-28 23:10:10]

lgtm

https://codereview.chromium.org/18055008/diff/3002/content/browser/accessibil...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/18055008/diff/3002/content/browser/accessibil...
content/browser/accessibility/browser_accessibility_win.cc:876: return E_FAIL;
nit: perhaps dcheck?  also below

[dmazzoni, 2013-06-28 23:34:51]

https://codereview.chromium.org/18055008/diff/3002/content/browser/accessibil...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/18055008/diff/3002/content/browser/accessibil...
content/browser/accessibility/browser_accessibility_win.cc:876: return E_FAIL;
On 2013/06/28 23:10:10, jam wrote:
> nit: perhaps dcheck?  also below

Done.

[commit-bot: I haz the power, 2013-06-28 23:36:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmazzoni@chromium.org/18055008/12001

[commit-bot: I haz the power, 2013-06-29 14:22:31]

Change committed as 209317