Android omnibox: Force paragraph direction to LTR.

Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
Committed: https://crrev.com/3bd33fee094e863e5496ac24714c558bd58d28ef
Cr-Commit-Position: refs/heads/master@{#400359}

[Matt Giuca, 2016-05-17 06:53:08]

Description was changed from

==========
Android Omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
==========

to

==========
Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
==========

[Matt Giuca, 2016-05-17 06:59:03]

mgiuca@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Matt Giuca, 2016-05-17 06:59:04]

See discussion on bug and https://codereview.chromium.org/1189553002 which this
is modelled off.

Since the URL bar is now always LTR, it makes sense for the suggestions to also
always be LTR, so removing all of the suggestions direction logic. Keeping the
test but simplifying it.

[Matt Giuca, 2016-05-17 08:13:49]

Updated the CL to correctly set alignment and suggestions view to match.

See https://crbug.com/609680#c14 for screenshots and some discussion (note the
paragraph about the blinking cursor).

[Ted C, 2016-05-18 17:01:01]

Historically we've got a bunch of bugs about the split cursor, and I'm going to
have a hard time thinking we should make that the default case for RTL.

Should we force LTR only when the URL isn't focused/taking input?  Then it won't
make it so wonky?

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
(right):

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:179:
setTextDirection(TEXT_DIRECTION_LTR);
Only available in API 17+

https://developer.android.com/reference/android/view/View.html#setTextDirecti...

Our wrapper for this:
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:180:
setTextAlignment(TEXT_ALIGNMENT_VIEW_START);
Same:

https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

[Matt Giuca, 2016-05-19 07:40:22]

On 2016/05/18 17:01:01, Ted C wrote:
> Historically we've got a bunch of bugs about the split cursor, and I'm going
to
> have a hard time thinking we should make that the default case for RTL.
> 
> Should we force LTR only when the URL isn't focused/taking input?  Then it
won't
> make it so wonky?

That's maybe a good solution. I think that's what MS Edge does (IIRC).

Thanks for the review. I'll think about this in more detail tomorrow (I don't
think this will land before branch point so I'll de-prioritize it).

> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> File chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
> (right):
> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:179:
> setTextDirection(TEXT_DIRECTION_LTR);
> Only available in API 17+
> 
>
https://developer.android.com/reference/android/view/View.html#setTextDirecti...
> 
> Our wrapper for this:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...
> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:180:
> setTextAlignment(TEXT_ALIGNMENT_VIEW_START);
> Same:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

[Matt Giuca, 2016-05-25 08:41:46]

I decided you were right. See comment on bug with attached screencasts:
https://bugs.chromium.org/p/chromium/issues/detail?id=609680#c19

[Matt Giuca, 2016-05-25 08:42:10]

I've uploaded the new patch set that controls those videos, but it isn't ready
for review yet.

[Matt Giuca, 2016-06-02 03:50:31]

Patchset #5 (id:80001) has been deleted

[Matt Giuca, 2016-06-09 08:39:13]

Hi Ted,

Note, I've reverted the changes to the suggestions view (since while the text is
editable, it can still be RTL, we keep the existing behaviour where the
suggestions can be right-aligned). PTAL.

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
(right):

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:179:
setTextDirection(TEXT_DIRECTION_LTR);
On 2016/05/18 17:01:01, Ted C wrote:
> Only available in API 17+
> 
>
https://developer.android.com/reference/android/view/View.html#setTextDirecti...
> 
> Our wrapper for this:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

Done.

https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:180:
setTextAlignment(TEXT_ALIGNMENT_VIEW_START);
On 2016/05/18 17:01:01, Ted C wrote:
> Same:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...

Done.

[Ted C, 2016-06-09 17:28:44]

On 2016/06/09 08:39:13, Matt Giuca wrote:
> Hi Ted,
> 
> Note, I've reverted the changes to the suggestions view (since while the text
is
> editable, it can still be RTL, we keep the existing behaviour where the
> suggestions can be right-aligned). PTAL.
> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> File chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
> (right):
> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:179:
> setTextDirection(TEXT_DIRECTION_LTR);
> On 2016/05/18 17:01:01, Ted C wrote:
> > Only available in API 17+
> > 
> >
>
https://developer.android.com/reference/android/view/View.html#setTextDirecti...
> > 
> > Our wrapper for this:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...
> 
> Done.
> 
>
https://codereview.chromium.org/1988553002/diff/60001/chrome/android/java/src...
> chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java:180:
> setTextAlignment(TEXT_ALIGNMENT_VIEW_START);
> On 2016/05/18 17:01:01, Ted C wrote:
> > Same:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/java/...
> 
> Done.

lgtm

[Matt Giuca, 2016-06-10 00:43:49]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-06-10 00:44:36]

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

[commit-bot: I haz the power, 2016-06-10 02:45:51]

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

[commit-bot: I haz the power, 2016-06-10 02:45:51]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[Matt Giuca, 2016-06-17 04:08:15]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-06-17 04:08:26]

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

[commit-bot: I haz the power, 2016-06-17 04:54:32]

Description was changed from

==========
Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
==========

to

==========
Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
==========

[commit-bot: I haz the power, 2016-06-17 04:54:33]

Committed patchset #10 (id:200001)

[commit-bot: I haz the power, 2016-06-17 04:56:15]

Description was changed from

==========
Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680
==========

to

==========
Android omnibox: Force paragraph direction to LTR.

This means that URLs will always be displayed in a left-to-right
context. Right-to-left runs are still rendered as RTL, but will not
cause the whole URL to flip around.

This fixes several spoofing concerns and makes Android's omnibox
consistent with the Views omnibox.

BUG=609680

Committed: https://crrev.com/3bd33fee094e863e5496ac24714c558bd58d28ef
Cr-Commit-Position: refs/heads/master@{#400359}
==========

[commit-bot: I haz the power, 2016-06-17 04:56:16]

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