New issue
Advanced search Search tips

Issue 749526 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

Refactor URL formatter https scheme strip option

Project Member Reported by cjgrant@chromium.org, Jul 27 2017

Issue description

As follow-on work to  issue 740112 , we need to complete a small refactor to the naming of URL formatter constants.  See details in that issue.

Summary of the required rework from tommycli@ (paraphrased):

----------
 1. Rename the "strip https scheme" option to remove "experimental" from the name.

 2. Figure out what kFormatUrlOmitAll now means. I suspect it really means "default" or "sane defaults" and should be renamed kFormatUrlOmitDefaults or something. This may take a bit of investigation into the callsites.

 3. Re-jigger the comments to make clear that the flags within the default set don't include the experimental ones and the HTTPS trimming.
----------

We deferred this work from M-61 to M-62 to minimize the code change merged to the M-61 branch.  This bug tracks the additional work required to make the change properly.
 
Is this fixed?
No; the CL is in review here:  https://chromium-review.googlesource.com/c/606991
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 10 2017

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

commit 91efeb34e296e3e501fe86bc74b4a4e40c91a58d
Author: Christopher Grant <cjgrant@chromium.org>
Date: Thu Aug 10 22:24:35 2017

URL formatter: Make OmitHTTPS non-experimental.

HTTPS scheme removal is now used in one non-experimental situation (VR),
so adjust its naming to match. Also, rename OmitAll to OmitDefaults, as
the OmitAll constant will be misleading otherwise.

BUG= 749526 

Change-Id: I3ecac2fab05ee63e10dc58e245381381d61fc833
Reviewed-on: https://chromium-review.googlesource.com/606991
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493564}
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/chrome/browser/dom_distiller/tab_utils_android.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/chrome/browser/ui/bookmarks/bookmark_utils.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/chrome/browser/vr/elements/url_bar_texture.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/chrome/browser/vr/elements/url_bar_texture_unittest.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/omnibox/browser/autocomplete_match.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/omnibox/browser/history_url_provider.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/omnibox/browser/search_provider.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/omnibox/browser/search_suggestion_parser.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/toolbar/toolbar_model_impl.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/url_formatter/elide_url.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/url_formatter/url_formatter.cc
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/url_formatter/url_formatter.h
[modify] https://crrev.com/91efeb34e296e3e501fe86bc74b4a4e40c91a58d/components/url_formatter/url_formatter_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment