Issue metadata
Sign in to add a comment
|
ElideUrl inconsistently chops off end of domain if URL has no path |
||||||||||||||||||||||||
Issue descriptionChrome Version: 61 OS: All What steps will reproduce the problem? Call ElideUrl on: 1. "http://xyz.google.com/foo" 2. "http://xyz.google.com" to a width that fits only part of the domain. What is the expected result? 1 elides to "…google.com…". 2 elides to "…google.com". What happens instead? 1 elides to "…google.com…". 2 elides to "xyz.google…". That's inconsistent, and although ElideUrl is not used in security contexts, it's still better to show the most significant parts of the domain ("google.com") rather than the least ("xyz.google"). This is caused by the fact that the algorithm uses a bunch of totally different code paths depending on whether there is a path or not. If there is no path, it just calls regular ElideText on the full URL, whereas if there is a path, it tries a much more complex algorithm culminating in chopping off the subdomain.
,
Jul 7 2017
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fae10d6348ac31702986c53e138a08a3c4c71b26 commit fae10d6348ac31702986c53e138a08a3c4c71b26 Author: Matt Giuca <mgiuca@chromium.org> Date: Wed Aug 16 07:13:39 2017 ElideUrl: Fixed inconsistent eliding of domains (depending on path). Previously chopped off the *end* of the domain if there was no path, but the *start* if there was a path. Now always chops off the start of the domain (i.e., the least significant part). Bug: 739636 Change-Id: Iea94c5b1c4bcbea2e78d45aa2d3fd12f4ab4858a Reviewed-on: https://chromium-review.googlesource.com/561029 Commit-Queue: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#494713} [modify] https://crrev.com/fae10d6348ac31702986c53e138a08a3c4c71b26/components/url_formatter/elide_url.cc [modify] https://crrev.com/fae10d6348ac31702986c53e138a08a3c4c71b26/components/url_formatter/elide_url_unittest.cc
,
Aug 16 2017
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3532a7b85cdd1b90274cb2170c654e943f32f569 commit 3532a7b85cdd1b90274cb2170c654e943f32f569 Author: Matt Giuca <mgiuca@chromium.org> Date: Fri Aug 18 10:04:47 2017 Revert "ElideUrl: Fixed inconsistent eliding of domains (depending on path)." This reverts commit fae10d6348ac31702986c53e138a08a3c4c71b26. Reason for revert: Introduces crash (see bug). Bug: 756717 Original change's description: > ElideUrl: Fixed inconsistent eliding of domains (depending on path). > > Previously chopped off the *end* of the domain if there was no path, but > the *start* if there was a path. Now always chops off the start of the > domain (i.e., the least significant part). > > Bug: 739636 > Change-Id: Iea94c5b1c4bcbea2e78d45aa2d3fd12f4ab4858a > Reviewed-on: https://chromium-review.googlesource.com/561029 > Commit-Queue: Matt Giuca <mgiuca@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Cr-Commit-Position: refs/heads/master@{#494713} TBR=pkasting@chromium.org,mgiuca@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 739636 Change-Id: I1ba9f736a8d570e036538b7aa49d064dfbf29fd9 Reviewed-on: https://chromium-review.googlesource.com/620546 Reviewed-by: Matt Giuca <mgiuca@chromium.org> Commit-Queue: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#495514} [modify] https://crrev.com/3532a7b85cdd1b90274cb2170c654e943f32f569/components/url_formatter/elide_url.cc [modify] https://crrev.com/3532a7b85cdd1b90274cb2170c654e943f32f569/components/url_formatter/elide_url_unittest.cc
,
Aug 18 2017
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49fcda1747f6236cc9410cc19dc139a679167969 commit 49fcda1747f6236cc9410cc19dc139a679167969 Author: Matt Giuca <mgiuca@chromium.org> Date: Tue Aug 29 07:16:12 2017 Reland "ElideUrl: Fixed inconsistent eliding of domains (depending on path)." This is a reland of fae10d6348ac31702986c53e138a08a3c4c71b26 The reland addresses https://crbug.com/756717 (which was introduced by the original CL) and adds a DCHECK and regression test for that issue. Original change's description: > ElideUrl: Fixed inconsistent eliding of domains (depending on path). > > Previously chopped off the *end* of the domain if there was no path, but > the *start* if there was a path. Now always chops off the start of the > domain (i.e., the least significant part). > > Bug: 739636 > Change-Id: Iea94c5b1c4bcbea2e78d45aa2d3fd12f4ab4858a > Reviewed-on: https://chromium-review.googlesource.com/561029 > Commit-Queue: Matt Giuca <mgiuca@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Cr-Commit-Position: refs/heads/master@{#494713} Bug: 739636 , 756717 Change-Id: I17484bbcf6a58df947f5660b0ee47e795ae5c0e6 Reviewed-on: https://chromium-review.googlesource.com/622290 Commit-Queue: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#498036} [modify] https://crrev.com/49fcda1747f6236cc9410cc19dc139a679167969/components/url_formatter/elide_url.cc [modify] https://crrev.com/49fcda1747f6236cc9410cc19dc139a679167969/components/url_formatter/elide_url_unittest.cc
,
Aug 29 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by mgiuca@chromium.org
, Jul 6 2017