Fix ChromeVox Next compile.

Fix ChromeVox Next compile.

Committed: https://crrev.com/f72920ef659207f761f8b8233fb597391fc609a8
Cr-Commit-Position: refs/heads/master@{#297541}

[David Tseng, 2014-09-30 21:49:46]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[dmazzoni, 2014-09-30 22:09:30]

lgtm

https://codereview.chromium.org/613223003/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py
(right):

https://codereview.chromium.org/613223003/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py:54:
[[CVoxPath('cvox2/background/loader.js')], _COMMON_EXTERNS],
nit: indentation

[David Tseng, 2014-09-30 22:16:42]

https://codereview.chromium.org/613223003/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py
(right):

https://codereview.chromium.org/613223003/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py:54:
[[CVoxPath('cvox2/background/loader.js')], _COMMON_EXTERNS],
On 2014/09/30 22:09:30, dmazzoni wrote:
> nit: indentation

Done.

[David Tseng, 2014-09-30 22:16:50]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-09-30 22:18:23]

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

[commit-bot: I haz the power, 2014-09-30 22:28:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-30 22:28:50]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[David Tseng, 2014-09-30 22:46:34]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-09-30 22:48:07]

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

[commit-bot: I haz the power, 2014-09-30 23:23:54]

Committed patchset #3 (id:30001) as 9266adb24a143ce733a61bb9abbc466d3ad1919f

[commit-bot: I haz the power, 2014-09-30 23:24:35]

Patchset 3 (id:??) landed as
https://crrev.com/f72920ef659207f761f8b8233fb597391fc609a8
Cr-Commit-Position: refs/heads/master@{#297541}

[Peter Lundblad, 2014-10-01 08:55:06]

plundblad@chromium.org changed reviewers:
+ plundblad@chromium.org

[Peter Lundblad, 2014-10-01 08:55:07]

Hi,

Thanks for adding compilation to cvoxnext!
I noticed this was submitted. Could you adress the below comments in
a follow-up cl?

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/common/chrome_extension_externs.js
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/chrome_extension_externs.js:1262:
* @param {string} eventType
Could this be refering to an EventType enum isntead?  It'd be great if the code
could be using an enum isntead of raw strings.

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:8: *
nit: why this extra blank line?

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:149:
current = current.role == 'inlineTextBox' ?
I think it is sad to move to literal strings away from actual symbolic names.
Why is that good?

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py:39:
CVoxPath('cvox2/background/externs.js'),
I dont' think we need to add this to chromevox classic, can it be split off into
its own list?

[David Tseng, 2014-10-01 16:18:14]

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/common/chrome_extension_externs.js
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/common/chrome_extension_externs.js:1262:
* @param {string} eventType
On 2014/10/01 08:55:07, Peter Lundblad wrote:
> Could this be refering to an EventType enum isntead?  It'd be great if the
code
> could be using an enum isntead of raw strings.

We don't want to include all of the EventTypes inside of externs. Totally
unmaintainable. Need to come up with a better way and definitely out of scope
for this cl.

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:8: *
On 2014/10/01 08:55:07, Peter Lundblad wrote:
> nit: why this extra blank line?

Will fix in subsequent cl; introduced to see if compliation actually worked (it
didn't at the time).

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:149:
current = current.role == 'inlineTextBox' ?
On 2014/10/01 08:55:07, Peter Lundblad wrote:
> I think it is sad to move to literal strings away from actual symbolic names.
> Why is that good?

Need to fix externs first. We already have *two* copies of event/role strings
floating around in idls, don't want another one yet in externs.

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py
(right):

https://codereview.chromium.org/613223003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/tools/check_chromevox.py:39:
CVoxPath('cvox2/background/externs.js'),
On 2014/10/01 08:55:07, Peter Lundblad wrote:
> I dont' think we need to add this to chromevox classic, can it be split off
into
> its own list?

Deleting in subsequent cl.

[David Tseng, 2014-10-01 16:59:53]

A revert of this CL (patchset #3 id:30001) has been created in
https://codereview.chromium.org/617323002/ by dtseng@chromium.org.

The reason for reverting is: Still causing errors in compile..

[David Tseng, 2014-10-01 17:13:20]

FYI; reland cl is here:
https://codereview.chromium.org/608853006/