Minimize the Classic API and enable it for Next

Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key echo, etc when braille mode is off.
BUG=690635, 690636, 618099
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2740793002
Cr-Commit-Position: refs/heads/master@{#456133}
Committed: https://chromium.googlesource.com/chromium/src/+/99e1012309d7f5f7eea0ff2157f06de806b3362c

[David Tseng, 2017-03-08 23:30:11]

Description was changed from

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=
==========

to

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-03-08 23:31:55]

Description was changed from

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=690635,690636
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-03-08 23:31:55]

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

[David Tseng, 2017-03-08 23:36:36]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 23:37:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Tseng, 2017-03-08 23:42:45]

Description was changed from

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=690635,690636
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=690635,690636,618099
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-03-09 00:26:36]

Should probably manually that Classic features are working
on non-Docs sites. Test features like find-in-page and
some event handlers

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/event_watcher.js
(left):

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/event_watcher.js:596:
cvox.ApiImplementation.syncToNode(
Are you sure it's a no-op to remove these?

I thought most uses of syncToNode were actually syncing ChromeVox to a
specific node and the only reason ApiImplementation was used was to have
a single shared implementation of that function.

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js
(right):

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js:91:
console.log('inst!'+scriptText);
remove debugging?

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js:96:
var p = document.head || document.body;
Give this a clearer name like scriptOwner or something like that?

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/widget.js
(left):

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/widget.js:117:
cvox.ApiImplementation.syncToNode(this.initialNode,
We definitely shouldn't just remove this one...

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs
(right):

https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs:567:
assertEquals('FORCE_next', bk.mode);
check capitalization

[commit-bot: I haz the power, 2017-03-09 00:29:54]

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

[commit-bot: I haz the power, 2017-03-09 00:29:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[David Tseng, 2017-03-09 19:34:35]

dmazzoni@chromium.org writes:

> Should probably manually that Classic features are working
> on non-Docs sites. Test features like find-in-page and
> some event handlers
>

Things worked manually, but yup, the tests were definitely catching the
lack of sync to node. Re-added.
>
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
> File
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/event_watcher.js
> (left):
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/event_watcher.js:596:
> cvox.ApiImplementation.syncToNode(
> Are you sure it's a no-op to remove these?
>

Moved from ApiImplementation to NavigationManager.

> I thought most uses of syncToNode were actually syncing ChromeVox to a
> specific node and the only reason ApiImplementation was used was to have
> a single shared implementation of that function.

Yes, somewhat misleading.
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
> File
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js
> (right):
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js:91:
> console.log('inst!'+scriptText);
> remove debugging?
>
Removed.
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/script_installer.js:96:
> var p = document.head || document.body;
> Give this a clearer name like scriptOwner or something like that?
>
Done.

>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
> File
> chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/widget.js
> (left):
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/widget.js:117:
> cvox.ApiImplementation.syncToNode(this.initialNode,
> We definitely shouldn't just remove this one...
>

See above.
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
> File
>
chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs
> (right):
>
>
https://codereview.chromium.org/2740793002/diff/1/chrome/browser/resources/ch...
>
chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs:567:
> assertEquals('FORCE_next', bk.mode);
> check capitalization
>Done


> https://codereview.chromium.org/2740793002/


-- 

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2017-03-09 19:38:34]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 19:39:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 20:30:30]

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

[commit-bot: I haz the power, 2017-03-09 20:30:30]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2017-03-10 19:11:08]

lgtm

[David Tseng, 2017-03-10 19:17:59]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2017-03-10 19:18:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 19:28:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489173479197650,
"parent_rev": "b6b3ddb97f0813f12cd304f3b125d17393920350", "commit_rev":
"99e1012309d7f5f7eea0ff2157f06de806b3362c"}

[commit-bot: I haz the power, 2017-03-10 19:28:56]

Description was changed from

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=690635,690636,618099
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Minimize the Classic API and enable it for Next

TEST=manual; verify Docs, Sheets, and Slides work with regard to whitespace, key
echo, etc when braille mode is off.
BUG=690635,690636,618099
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2740793002
Cr-Commit-Position: refs/heads/master@{#456133}
Committed:
https://chromium.googlesource.com/chromium/src/+/99e1012309d7f5f7eea0ff2157f0...
==========

[commit-bot: I haz the power, 2017-03-10 19:28:57]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/99e1012309d7f5f7eea0ff2157f0...