New issue
Advanced search Search tips

Issue 753789 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Components:
EstimatedDays: 3
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR

Blocking:
issue 731919



Sign in to add a comment

Support URL elision with coloring

Project Member Reported by vollick@chromium.org, Aug 9 2017

Issue description

We currently use URL elision for things like the pale beige loading indicator that appears at the bottom of a page on desktop. This code does not separate the URL into its constituent components for coloring since the elided URLs are not colored.

We would like to support both coloring and elision, so we will have to augment the elision code to handle both.
 
Components: UI>Browser>VR
Labels: Proj-VR-Shell
Owner: cjgrant@chromium.org
Status: Assigned (was: Available)
There is a bug open that tracks this initiative.  Taking the bug for now to track down other info and associate it here.
Owner: ----
Status: Available (was: Assigned)
When considering URL eliding for VR, please fully read  issue 735123 , and the bug it blocks.
Status: Started (was: Available)
Based on a prioritization chat with Ian, I'm plucking this as my next task.
Cc: -cjgrant@chromium.org
Owner: cjgrant@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 6 2017

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

commit 3e3149a88fef77fd4a11aba2a881e8b43941d25e
Author: Christopher Grant <cjgrant@chromium.org>
Date: Wed Sep 06 14:54:39 2017

ElideUrl: Refactor tests to use a progressive elision approach.

Several elision test cases specify multiple progressively shorter
versions of the same original URL. With this change, we'll test that
progression more thoroughly by specifying the expected sequence of
consecutive results that should be generated as available space
decreases.

This helps eliminate the risk of a broken corner case.

BUG= 753789 

Change-Id: Ib727295522f588f5e8b658661d2314587aa61844
Reviewed-on: https://chromium-review.googlesource.com/643153
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499962}
[modify] https://crrev.com/3e3149a88fef77fd4a11aba2a881e8b43941d25e/components/url_formatter/elide_url_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 6 2017

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

commit a8589ec1f0498721dd0b563106a84bd963c915cb
Author: Jayson Adams <shrike@chromium.org>
Date: Wed Sep 06 16:12:43 2017

Revert "ElideUrl: Refactor tests to use a progressive elision approach."

This reverts commit 3e3149a88fef77fd4a11aba2a881e8b43941d25e.

Reason for revert: components_unittests TextEliderTest.TestGeneralEliding failing on macOS 10.10, 10.11, and 10.12, and browser_side_navigation_components_unittests TextEliderTest.TestGeneralEliding failing on 10.11 and 10.12

https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.10%20Tests/builds/23322

https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.11%20Tests

https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.12%20Tests

https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.11%20Tests

https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.12%20Tests

Original change's description:
> ElideUrl: Refactor tests to use a progressive elision approach.
> 
> Several elision test cases specify multiple progressively shorter
> versions of the same original URL. With this change, we'll test that
> progression more thoroughly by specifying the expected sequence of
> consecutive results that should be generated as available space
> decreases.
> 
> This helps eliminate the risk of a broken corner case.
> 
> BUG= 753789 
> 
> Change-Id: Ib727295522f588f5e8b658661d2314587aa61844
> Reviewed-on: https://chromium-review.googlesource.com/643153
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Reviewed-by: Matt Giuca <mgiuca@chromium.org>
> Commit-Queue: Christopher Grant <cjgrant@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#499962}

TBR=msw@chromium.org,tommycli@chromium.org,mgiuca@chromium.org,cjgrant@chromium.org

Change-Id: I6ec725f1496aa84019489dc3c8f0026a50e6dfdb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  753789 
Reviewed-on: https://chromium-review.googlesource.com/653197
Reviewed-by: Jayson Adams <shrike@chromium.org>
Commit-Queue: Jayson Adams <shrike@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499979}
[modify] https://crrev.com/a8589ec1f0498721dd0b563106a84bd963c915cb/components/url_formatter/elide_url_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 7 2017

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

commit f205fcfb6248e13bc1d4a8a953932c5fa1b720c8
Author: Christopher Grant <cjgrant@chromium.org>
Date: Thu Sep 07 21:25:59 2017

ElideUrl: Re-land progressive elision approach.

Several elision test cases specify multiple progressively shorter
versions of the same original URL. With this change, we'll test that
progression more thoroughly by specifying the expected sequence of
consecutive results that should be generated as available space
decreases.

This helps eliminate the risk of a broken corner case.

BUG= 753789 

Change-Id: I6e7d8fcece3895b5b1c2595b2afacf34adea78b5
Reviewed-on: https://chromium-review.googlesource.com/655397
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500385}
[modify] https://crrev.com/f205fcfb6248e13bc1d4a8a953932c5fa1b720c8/components/url_formatter/elide_url_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2017

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

commit ed6173edcc8f043217548894f5efd0f1418bcaab
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Sep 08 01:35:13 2017

Revert "ElideUrl: Re-land progressive elision approach."

This reverts commit f205fcfb6248e13bc1d4a8a953932c5fa1b720c8.

Reason for revert: TestGeneralEliding fails on Mac10.10 bot.

Bug:  763202 

Original change's description:
> ElideUrl: Re-land progressive elision approach.
> 
> Several elision test cases specify multiple progressively shorter
> versions of the same original URL. With this change, we'll test that
> progression more thoroughly by specifying the expected sequence of
> consecutive results that should be generated as available space
> decreases.
> 
> This helps eliminate the risk of a broken corner case.
> 
> BUG= 753789 
> 
> Change-Id: I6e7d8fcece3895b5b1c2595b2afacf34adea78b5
> Reviewed-on: https://chromium-review.googlesource.com/655397
> Commit-Queue: Christopher Grant <cjgrant@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#500385}

TBR=tommycli@chromium.org,cjgrant@chromium.org,shrike@chromium.org

Change-Id: Ia6008f6f33c79a6368919b0a4324bdda1fd291bd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  753789 
Reviewed-on: https://chromium-review.googlesource.com/656797
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500476}
[modify] https://crrev.com/ed6173edcc8f043217548894f5efd0f1418bcaab/components/url_formatter/elide_url_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 12 2017

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

commit 5ed8d110614e627a3b7ff828bd2eaf7bad5a4135
Author: Christopher Grant <cjgrant@chromium.org>
Date: Tue Sep 12 20:00:06 2017

Reland "ElideUrl: Refactor tests to use a progressive elision approach."

This is a reland of 3e3149a88fef77fd4a11aba2a881e8b43941d25e
Original change's description:
> ElideUrl: Refactor tests to use a progressive elision approach.
> 
> Several elision test cases specify multiple progressively shorter
> versions of the same original URL. With this change, we'll test that
> progression more thoroughly by specifying the expected sequence of
> consecutive results that should be generated as available space
> decreases.
> 
> This helps eliminate the risk of a broken corner case.
> 
> BUG= 753789 
> 
> Change-Id: Ib727295522f588f5e8b658661d2314587aa61844
> Reviewed-on: https://chromium-review.googlesource.com/643153
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Reviewed-by: Matt Giuca <mgiuca@chromium.org>
> Commit-Queue: Christopher Grant <cjgrant@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#499962}

TBR=tommycli@chromium.org

Bug:  753789 
Change-Id: Ief5f6c029f93282de64d3946266b8608620aff80
Reviewed-on: https://chromium-review.googlesource.com/663677
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501369}
[modify] https://crrev.com/5ed8d110614e627a3b7ff828bd2eaf7bad5a4135/components/url_formatter/elide_url_unittest.cc

Labels: -M-62 M-63
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 9 2017

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

commit 6871e121336ff1854382f923c307d31f25cf4e50
Author: Christopher Grant <cjgrant@chromium.org>
Date: Mon Oct 09 15:47:50 2017

Add a URL elision method tailored for a URL bar

VR mode currently shows a read-only URL bar. We currently bail from VR
if the hostname doesn't fit on the bar, but we should be eliding
instead.

Enamel's current guidance for this scenario is to elide from
the right until we've chopped the path and related parts, then continue
to elide from the front, preserving the TLD as long as possible. This is
different from other elision helpers currently available.

This method, and the original ElideUrl() method, should converge
incrementally to a single algorithm.

BUG= 753789 

Change-Id: I19464f00df8f3e87743bbb19f857210164f810c3
Reviewed-on: https://chromium-review.googlesource.com/627121
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507382}
[modify] https://crrev.com/6871e121336ff1854382f923c307d31f25cf4e50/components/url_formatter/BUILD.gn
[modify] https://crrev.com/6871e121336ff1854382f923c307d31f25cf4e50/components/url_formatter/elide_url.cc
[modify] https://crrev.com/6871e121336ff1854382f923c307d31f25cf4e50/components/url_formatter/elide_url.h
[modify] https://crrev.com/6871e121336ff1854382f923c307d31f25cf4e50/components/url_formatter/elide_url_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 13 2017

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

commit bd541e56e412e01331cef3c00c51b8f6925854e3
Author: Christopher Grant <cjgrant@chromium.org>
Date: Fri Oct 13 20:59:41 2017

VR: Elide URLs rather than bailing from VR.

Prior to this change, if the host portion of a URL couldn't fit into
the VR URL bar, we'd bail from VR to protect the user. Now that we can
elide by showing the TLD and TLD+1 as long as possible, we no longer
need to bail.

Bug= 753789 

R=msw

Change-Id: I7abc9c01eb2061a9935f34a805deb8726ceda042
Reviewed-on: https://chromium-review.googlesource.com/710226
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508812}
[modify] https://crrev.com/bd541e56e412e01331cef3c00c51b8f6925854e3/chrome/browser/vr/elements/url_bar_texture.cc
[modify] https://crrev.com/bd541e56e412e01331cef3c00c51b8f6925854e3/chrome/browser/vr/elements/url_bar_texture_unittest.cc
[modify] https://crrev.com/bd541e56e412e01331cef3c00c51b8f6925854e3/components/url_formatter/elide_url.cc
[modify] https://crrev.com/bd541e56e412e01331cef3c00c51b8f6925854e3/components/url_formatter/elide_url.h
[modify] https://crrev.com/bd541e56e412e01331cef3c00c51b8f6925854e3/components/url_formatter/elide_url_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment