Workaround for navigating by word in the omnibox on Windows.

Workaround for navigating by word in the omnibox on Windows.

Until we merge content accessibility code with Views accessibility.
BUG=103180
R=dmazzoni@chromium.org
TESTED=manually with NVDA, unit test

Committed: https://crrev.com/2a0540405233dafcde7c7891516f43bed0a48b2a
Cr-Commit-Position: refs/heads/master@{#380267}

[dmazzoni, 2016-03-08 19:16:07]

lgtm

Looks like a great short-term fix!

[chromium-reviews, 2016-03-08 21:40:04]

Sorry, I added a unit test.
Could you also check my modification to the build.gn file since I can't 
test gn on Windows?

-- 
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.

[nektarios, 2016-03-08 23:01:15]

Description was changed from

==========
Workaround for navigating by word in the omnibox on Windows.

Until we merge content accessibility code with Views accessibility.
BUG=103180
R=dmazzoni@chromium.org
TESTED=manually with NVDA
==========

to

==========
Workaround for navigating by word in the omnibox on Windows.

Until we merge content accessibility code with Views accessibility.
BUG=103180
R=dmazzoni@chromium.org
TESTED=manually with NVDA, unit test
==========

[nektarios, 2016-03-08 23:01:15]

nektar@chromium.org changed reviewers:
+ jshin@chromium.org

[nektarios, 2016-03-08 23:02:28]

@jshin
Please approve ui/accessibility to depend on base/i18n.
We need this in order to do word navigation in the omnibox for screen reader
users.

[nektarios, 2016-03-09 02:51:39]

nektar@chromium.org changed reviewers:
+ mark@chromium.org

[nektarios, 2016-03-09 02:52:23]

@mark
Could you please approve this change to depend on base/i18n?

[Mark Mentovai, 2016-03-09 03:00:31]

https://codereview.chromium.org/1780473002/diff/60001/ui/accessibility/DEPS
File ui/accessibility/DEPS (right):

https://codereview.chromium.org/1780473002/diff/60001/ui/accessibility/DEPS#n...
ui/accessibility/DEPS:3: "+ui/base/i18n",
You should be depending on base/i18n, not the nonexistent ui/base/i18n.

https://codereview.chromium.org/1780473002/diff/60001/ui/accessibility/DEPS#n...
ui/accessibility/DEPS:3: "+ui/base/i18n",
I find it weird that things in ui/accessibility are already #including files in
base and the target is already depending on base, but this DEPS file doesn’t
mention it at all.

[chromium-reviews, 2016-03-09 17:47:39]

@mark
Fixed. Also sorted in alphabetical order. Sorry for the obvious mistake. 
Too much copy pasting.
Could you please review my changes to build.gn? I have no way of testing 
this on Windows.

-- 
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.

[Mark Mentovai, 2016-03-09 18:42:35]

LG otherwise

https://codereview.chromium.org/1780473002/diff/80001/ui/accessibility/BUILD.gn
File ui/accessibility/BUILD.gn (right):

https://codereview.chromium.org/1780473002/diff/80001/ui/accessibility/BUILD....
ui/accessibility/BUILD.gn:77: "//base_i18n",
I don’t actually know if this is correct. I don’t know much about gn or our gn
build. This seems a little awkward because I thought that these were supposed to
be paths in the Chrome tree, and while we have base and ui at the root, we don’t
have base_i18n.

Based on
https://code.google.com/p/chromium/codesearch#search/&q=file:BUILD.gn%20//bas...,
I would expect to see this as //base:i18n

https://codereview.chromium.org/1780473002/diff/80001/ui/accessibility/DEPS
File ui/accessibility/DEPS (right):

https://codereview.chromium.org/1780473002/diff/80001/ui/accessibility/DEPS#n...
ui/accessibility/DEPS:3: "+base/i18n",
Shouldn’t be necessary if you’re adding base already.

But what I kind of meant was that no checks were violated in the past with
ui/accessibility depending on base, so why did you need to add something to
depend on base/i18n? Was there a presubmit error? Did you run this past a try
bot or try to commit it and fail a check? If not, I’d just as soon try this
without.

[chromium-reviews, 2016-03-09 20:53:51]

Okay, removed base from deps and modified build.gn.
I'll try testing with the bots and if it passes submit.

-- 
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.

[nektarios, 2016-03-09 20:57:15]

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

[commit-bot: I haz the power, 2016-03-09 20:59:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1780473002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1780473002/100001

[Mark Mentovai, 2016-03-09 21:41:08]

LGTM

[commit-bot: I haz the power, 2016-03-09 23:59:59]

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

[commit-bot: I haz the power, 2016-03-10 00:00:00]

Dry run: This issue passed the CQ dry run.

[nektarios, 2016-03-10 00:01:54]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-10 00:01:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/1780473002/#ps100001
(title: "Removed base from deps and fixed build.gn file (hopefully).")

[commit-bot: I haz the power, 2016-03-10 00:02:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1780473002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1780473002/100001

[commit-bot: I haz the power, 2016-03-10 00:09:18]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-10 00:10:45]

Description was changed from

==========
Workaround for navigating by word in the omnibox on Windows.

Until we merge content accessibility code with Views accessibility.
BUG=103180
R=dmazzoni@chromium.org
TESTED=manually with NVDA, unit test
==========

to

==========
Workaround for navigating by word in the omnibox on Windows.

Until we merge content accessibility code with Views accessibility.
BUG=103180
R=dmazzoni@chromium.org
TESTED=manually with NVDA, unit test

Committed: https://crrev.com/2a0540405233dafcde7c7891516f43bed0a48b2a
Cr-Commit-Position: refs/heads/master@{#380267}
==========

[commit-bot: I haz the power, 2016-03-10 00:10:46]

Patchset 6 (id:??) landed as
https://crrev.com/2a0540405233dafcde7c7891516f43bed0a48b2a
Cr-Commit-Position: refs/heads/master@{#380267}