CC'ing a few people who have touched ElideUrl to help triage.

Ouch, this is bad. I looked at the code in ElideUrl: it's a horrible rats-nest of special cases and applies different logic to the whole URL depending on things like the presence or absence of a path. And that's just one of a handful of different URL eliding functions in that file.

This particular bug is that if the path is empty, it accidentally sets "url_path" to the entire URL string, then concatenates it onto the end of the domain, then elides it.

I also noticed another bug:  Issue 739636  which I'll fix separately.

Fix for this issue: https://chromium-review.googlesource.com/c/561040/

Note that ElideUrl is only used in two places in Chrome: the status / hover bubble, and the bookmark tooltip. If you are using it for anything that informs the user where they currently are (and therefore would be considered a security surface), you should use FormatUrlForSecurityDisplay which is a completely different algorithm.

Thanks for the fix!  Based on your advice, I need to ask some questions.

I was looking into this function for use with Chrome VR.  There, we're drawing a URL bar without a lot of space (similar to Clank on a phone).  However, we're drawing the bar natively, as VR has no ties to the Android UI framework.  Unrelated crbug/735759 has some screenshots for reference.

We looked at how URLs are elided, and see that:

- Desktop elides from the right, non-selectively.
- Android simply scrolls the URL text field such that the end of the domain is on the right edge of the field (no "eliding").

Based on advice from Enamel, we really hope to offer something like in the doc below (see the section on specific Do's and Don'ts of URL eliding):

https://www.chromium.org/Home/chromium-security/enamel#TOC-Presenting-Origins

It *looks* like we want basically what ElideURL does, except that ElideUrl doesn't maintain the ability to emphasize resulting text (it doesn't return character ranges for host, path, etc).  On the other hand, FormatUrlForSecurityDisplay() doesn't do eliding.

For now, we actually bail out of VR if we can't display the complete TLD, but this is not an acceptable long-term solution, so we need to settle on a decent elision system (whatever it ends up being).

Feel free to ping offline, or on here, but either way, I'm hoping we can talk.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51f68cc40158272d068bac1ed6638d2a9ca6b82d

commit 51f68cc40158272d068bac1ed6638d2a9ca6b82d
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Jul 07 01:34:57 2017

ElideUrl: Fixed bug where the scheme can be added to the end of the URL.

If a URL has no path, the logic in ElideUrl erroneously considers the
entire URL to be the path. Later in the algorithm, it appends the path
to the end of the domain. This means a URL like "https://www.abc.com"
could be elided as "…abc.comhttps://…". Fixed this logic.

Added a test case for this, and also more thorough tests for eliding
various lengths of basic URLs.

Bug:  735123 
Change-Id: I39e0a37d62728f6592454990140534dda4445cfa
Reviewed-on: https://chromium-review.googlesource.com/561040
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484784}
[modify] https://crrev.com/51f68cc40158272d068bac1ed6638d2a9ca6b82d/components/url_formatter/elide_url.cc
[modify] https://crrev.com/51f68cc40158272d068bac1ed6638d2a9ca6b82d/components/url_formatter/elide_url_unittest.cc

#4 OK I didn't realise FormatUrlForSecurityDisplay doesn't elide. I'm not actually on the security team so I'm not really qualified to talk about secure URL eliding. It does seem strange that we have this function for eliding but not on secure surfaces. We should probably focus on a single eliding implementation and make it acceptable for security.

I've been looking at FormatUrl over the past day and it does seem reasonably equipped for security (e.g., it elides the left part of the domain if it has to), but it has a LOT of edge cases where this won't happen. I think if we simplify it and rework it, it could be usable in a secure context.

For now, I think it's OK to use ElideUrl in VR but be aware that there are edge cases where the most significant parts of a domain may be cut off.

Oh also be aware that ElideUrl cuts off the "https" (even for secure origins) so that may not be great either.

mgiuca@, for VR purposes, this elision work has bubbled up to the surface.  I'm starting the work of figuring out how we'll elide our URLs, securely, for VR.

At a high-level, it feels like I should start with ElideURL, and do exactly what you said:  "We should probably focus on a single eliding implementation and make it acceptable for security."

Whether or not the end result is used by desktop or clank, we should have a method that's secure, can elide, and can return metadata usable for highlighting and emphasis.

If it's cool with you, I'll start dissecting the "horrible rats-nest" and see what I can do.  With any luck, I can feed you back an implementation that's cleaner, Enamel-appeasing and usable by (at least) VR.  Let me know!