Default page title (the page's URL) is now in an LTR embedding.

Default page title (the page's URL) is now in an LTR embedding.

Only affects URLs with strong right-to-left characters.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[Matt Giuca, 2016-07-22 01:57:46]

Description was changed from

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
==========

to

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Matt Giuca, 2016-07-22 03:03:12]

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

[Matt Giuca, 2016-07-22 03:03:13]

Hi, PTAL (see bug for detailed description of the problem).

[Matt Giuca, 2016-07-22 03:04:49]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-07-22 03:04:51]

The CQ bit was unchecked by mgiuca@chromium.org

[Matt Giuca, 2016-07-22 03:04:57]

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

[commit-bot: I haz the power, 2016-07-22 03:05:24]

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, 2016-07-22 03:05:25]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-22 03:30:37]

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

[commit-bot: I haz the power, 2016-07-22 03:30:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[clamy, 2016-07-22 14:05:16]

Thanks! Lgtm.

[Matt Giuca, 2016-07-26 07:28:35]

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

[commit-bot: I haz the power, 2016-07-26 07:28:50]

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, 2016-07-26 07:28:51]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-26 07:39:40]

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

[commit-bot: I haz the power, 2016-07-26 07:39:42]

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

[Matt Giuca, 2016-07-27 05:52:28]

Description was changed from

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
==========

[Matt Giuca, 2016-07-27 08:23:03]

Description was changed from

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
==========

to

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Matt Giuca, 2016-07-27 08:33:54]

Patchset #6 (id:100001) has been deleted

[Matt Giuca, 2016-07-27 08:34:05]

Patchset #6 (id:120001) has been deleted

[Matt Giuca, 2016-07-27 08:35:14]

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

[commit-bot: I haz the power, 2016-07-27 08:35:29]

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

[Matt Giuca, 2016-07-27 09:04:11]

Hi again clamy@,

Sorry to do this; I realised I broke a lot of tests and the change I was making
was a little over-zealous (changing ALL titles). In particular, that it would
break the Extensions API chrome.tabs, which allows searching and reading tab
titles.

PTAL at the new patch. Here, I am ONLY adding the LTR embedding if there are
strong RTL characters in the string. This has the same visual effect, but leaves
all non-RTL titles alone. The tests (which primarily use LTR / ASCII titles) are
unaffected.

This *still* technically breaks the chrome.tabs API, as you will get different
results on tabs with RTL characters in them. But I think it is correct to return
strings with LTR markup characters in them, in that API, for tabs with RTL URLs.
Let me know what you think.

[Matt Giuca, 2016-07-27 09:04:44]

Description was changed from

==========
Default page title (the page's URL) is now in an LTR embedding.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Default page title (the page's URL) is now in an LTR embedding.

Only affects URLs with strong right-to-left characters.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-07-27 09:25:16]

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

[commit-bot: I haz the power, 2016-07-27 09:25:17]

Dry run: This issue passed the CQ dry run.

[clamy, 2016-07-27 16:38:20]

Could you add an extension owner to the review, so that they can be informed of
the issue?
Note: I'll be away for 2 weeks, so you will need another content/ OWNER to
approve the patch.

[Matt Giuca, 2016-07-28 01:59:50]

mgiuca@chromium.org changed reviewers:
+ alexmos@chromium.org, reillyg@chromium.org

[Matt Giuca, 2016-07-28 01:59:51]

alexmos: Please take over from clamy@ who is OOO.

reillyg: For extensions API owners. Although I am not directly touching
extensions API code, this will affect the chrome.tabs API by changing the title
of tabs that have RTL characters in their URL, and no explicit title in the
document. Should be a very small set, unlikely to break any extensions in
practice. Are you OK with this?

[Reilly Grant (use Gerrit), 2016-07-28 17:58:11]

reillyg@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Reilly Grant (use Gerrit), 2016-07-28 17:58:12]

The change sounds reasonable to me but Devlin should make the decision about the
extensions API effects.

[alexmos, 2016-07-28 21:17:50]

On 2016/07/28 01:59:51, Matt Giuca wrote:
> alexmos: Please take over from clamy@ who is OOO.

frame_host/ LGTM but please wait for Devlin to ok the effect on chrome.tabs API.

[Devlin, 2016-07-28 21:24:22]

On 2016/07/27 09:04:11, Matt Giuca wrote:
> This *still* technically breaks the chrome.tabs API, as you will get different
> results on tabs with RTL characters in them. But I think it is correct to
return
> strings with LTR markup characters in them, in that API, for tabs with RTL
URLs.
> Let me know what you think.

Sorry, I'm having a bit of a hard time following this.  Can you give concrete
examples of what would be returned before vs now for searching titles with RTL
characters?

[Matt Giuca, 2016-07-29 01:36:12]

On 2016/07/28 21:24:22, Devlin wrote:
> On 2016/07/27 09:04:11, Matt Giuca wrote:
> > This *still* technically breaks the chrome.tabs API, as you will get
different
> > results on tabs with RTL characters in them. But I think it is correct to
> return
> > strings with LTR markup characters in them, in that API, for tabs with RTL
> URLs.
> > Let me know what you think.
> 
> Sorry, I'm having a bit of a hard time following this.  Can you give concrete
> examples of what would be returned before vs now for searching titles with RTL
> characters?

So this only affects titles made from a URL (i.e., the page title is blank so we
set the title to the page's URL).

If the URL is something like "http://جوجل.com", we will drop the "http://" and
decorate it with <U+202A> and <U+202C>, so the final title will be:
"<U+202A>جوجل.com<U+202C>"
where previously it would have been:
"جوجل.com"

Those special Unicode characters are included in the title returned by query()
because they are part of the title. I don't think extensions should be relying
on this, because this is equivalent to the web page itself setting its own title
with those special characters in it. But note that this also affects query
globbing. Previously a query for "*.com" would return the above page, whereas
now that query will not match, because there is another character after the
".com".

[Devlin, 2016-07-29 15:12:55]

On 2016/07/29 01:36:12, Matt Giuca wrote:
> On 2016/07/28 21:24:22, Devlin wrote:
> > On 2016/07/27 09:04:11, Matt Giuca wrote:
> > > This *still* technically breaks the chrome.tabs API, as you will get
> different
> > > results on tabs with RTL characters in them. But I think it is correct to
> > return
> > > strings with LTR markup characters in them, in that API, for tabs with RTL
> > URLs.
> > > Let me know what you think.
> > 
> > Sorry, I'm having a bit of a hard time following this.  Can you give
concrete
> > examples of what would be returned before vs now for searching titles with
RTL
> > characters?
> 
> So this only affects titles made from a URL (i.e., the page title is blank so
we
> set the title to the page's URL).
> 
> If the URL is something like "http://جوجل.com", we will drop the "http://" and
> decorate it with <U+202A> and <U+202C>, so the final title will be:
> "<U+202A>جوجل.com<U+202C>"
> where previously it would have been:
> "جوجل.com"
> 
> Those special Unicode characters are included in the title returned by query()
> because they are part of the title. I don't think extensions should be relying
> on this, because this is equivalent to the web page itself setting its own
title
> with those special characters in it. But note that this also affects query
> globbing. Previously a query for "*.com" would return the above page, whereas
> now that query will not match, because there is another character after the
> ".com".

Thanks for the explanation.  Agreed that extensions shouldn't be relying on
this.

lgtm

[Matt Giuca, 2016-08-01 03:36:42]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-08-01 03:36:43]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/2176463002/#ps140001
(title: "Backing out; only wrapping if URL contains RTL characters.")

[commit-bot: I haz the power, 2016-08-01 03:36:49]

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

[Matt Giuca, 2016-08-01 03:36:55]

On 2016/07/29 15:12:55, Devlin wrote:
> On 2016/07/29 01:36:12, Matt Giuca wrote:
> > On 2016/07/28 21:24:22, Devlin wrote:
> > > On 2016/07/27 09:04:11, Matt Giuca wrote:
> > > > This *still* technically breaks the chrome.tabs API, as you will get
> > different
> > > > results on tabs with RTL characters in them. But I think it is correct
to
> > > return
> > > > strings with LTR markup characters in them, in that API, for tabs with
RTL
> > > URLs.
> > > > Let me know what you think.
> > > 
> > > Sorry, I'm having a bit of a hard time following this.  Can you give
> concrete
> > > examples of what would be returned before vs now for searching titles with
> RTL
> > > characters?
> > 
> > So this only affects titles made from a URL (i.e., the page title is blank
so
> we
> > set the title to the page's URL).
> > 
> > If the URL is something like "http://جوجل.com", we will drop the "http://"
and
> > decorate it with <U+202A> and <U+202C>, so the final title will be:
> > "<U+202A>جوجل.com<U+202C>"
> > where previously it would have been:
> > "جوجل.com"
> > 
> > Those special Unicode characters are included in the title returned by
query()
> > because they are part of the title. I don't think extensions should be
relying
> > on this, because this is equivalent to the web page itself setting its own
> title
> > with those special characters in it. But note that this also affects query
> > globbing. Previously a query for "*.com" would return the above page,
whereas
> > now that query will not match, because there is another character after the
> > ".com".
> 
> Thanks for the explanation.  Agreed that extensions shouldn't be relying on
> this.
> 
> lgtm

Thanks for the detailed look!

[commit-bot: I haz the power, 2016-08-01 03:39:19]

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

[commit-bot: I haz the power, 2016-08-01 03:39:21]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Matt Giuca, 2016-08-01 05:34:11]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-08-01 05:34:12]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org,
rdevlin.cronin@chromium.org, alexmos@chromium.org
Link to the patchset: https://codereview.chromium.org/2176463002/#ps160001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-08-01 05:34:27]

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, 2016-08-01 06:50:42]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2016-08-01 06:51:56]

Description was changed from

==========
Default page title (the page's URL) is now in an LTR embedding.

Only affects URLs with strong right-to-left characters.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Default page title (the page's URL) is now in an LTR embedding.

Only affects URLs with strong right-to-left characters.

This fixes the case where the URL contains RTL characters, causing the
components of the URL to be rendered from right to left. The display is
now consistent with the Omnibox display of URLs and compliant with RFC
3987.

BUG=630481
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/c2099b61204ff4d273fdf9fc136bcb02d76093f6
Cr-Commit-Position: refs/heads/master@{#408920}
==========

[commit-bot: I haz the power, 2016-08-01 06:51:57]

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