New issue
Advanced search Search tips

Issue 682666 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

If distilled page leads to a redirection, the omnibox should display the final URL

Project Member Reported by olivierrobin@chromium.org, Jan 19 2017

Issue description

If adding goo.gl/6kK6uv tot he Reading List (via context menu or share extension), and if the entry is distilled, the omnibox should show https://fr.wikipedia.org/wiki/Google when opening the offline page.
 
Cc: mard...@chromium.org
Agreed. What got distilled is the final URL of the entry and not the transient redirection so the final URL should be displayed. 
Thanks.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 20 2017

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

commit 695d0c65669b7af00f8daec96d497519c0e0cb46
Author: olivierrobin <olivierrobin@chromium.org>
Date: Fri Jan 20 15:42:54 2017

Store the distilled_url in Reading List entry in Reading List on iOS.

The URL of the distilled page is not necessarily the URL of the Reading
List entry (in case of redirection).
Store the URL of the page that was distilled in the database.
This CL adds the field in ReadingListEntry.
This field will be used in a next CL.

BUG= 682666 

Review-Url: https://codereview.chromium.org/2647763005
Cr-Commit-Position: refs/heads/master@{#445063}

[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/proto/reading_list.proto
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_entry.cc
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_entry.h
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_entry_unittest.cc
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_model.h
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_model_impl.cc
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_model_impl.h
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/components/reading_list/ios/reading_list_model_unittest.mm
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/695d0c65669b7af00f8daec96d497519c0e0cb46/ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 20 2017

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

commit 69e7e3b844f1cedf53701f07bf0d3219fc2d3176
Author: olivierrobin <olivierrobin@chromium.org>
Date: Fri Jan 20 16:29:52 2017

Store distilled URL during distillation in Reading List on iOS

The URL of the distilled page is not necessarily the URL of the Reading
List entry (incase of redirection).
If the distillation is done on a redirected page, store it along the
distilled_path.

I also filed crbug.com/682705 to have the "redirected_url" directly in the proto file
and avoid having to take it via callback.

BUG= 682666 

Review-Url: https://codereview.chromium.org/2646993002
Cr-Commit-Position: refs/heads/master@{#445071}

[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/components/dom_distiller/ios/distiller_page_ios.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/dom_distiller/distiller_viewer.cc
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/dom_distiller/distiller_viewer.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_distiller_page.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/reading_list_download_service.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/url_downloader.cc
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/url_downloader.h
[modify] https://crrev.com/69e7e3b844f1cedf53701f07bf0d3219fc2d3176/ios/chrome/browser/reading_list/url_downloader_unittest.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 20 2017

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

commit 457f401daa5e0667546ab2a96afea821c6e5d88f
Author: olivierrobin <olivierrobin@chromium.org>
Date: Fri Jan 20 16:53:26 2017

[Reading List iOS] Display distilled URL instead of entry URL in omnibox

When displaying an offline page, the URL must be the actual distilled
URL and not the Reading List entry URL (which can be a short URL like
goo.gl/...).

BUG= 682666 

Review-Url: https://codereview.chromium.org/2644293002
Cr-Commit-Position: refs/heads/master@{#445077}

[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/reading_list/offline_url_utils.cc
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/reading_list/offline_url_utils.h
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/reading_list/offline_url_utils_unittest.cc
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/ui/reading_list/offline_page_native_content.mm
[modify] https://crrev.com/457f401daa5e0667546ab2a96afea821c6e5d88f/ios/chrome/browser/ui/reading_list/offline_page_native_content_unittest.mm

Status: Fixed (was: Started)
Labels: ReleaseBlock-Stable Merge-Request-57
Project Member

Comment 8 by sheriffbot@chromium.org, Jan 21 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 23 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c

commit 7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Mon Jan 23 15:20:19 2017

Store the distilled_url in Reading List entry in Reading List on iOS.

The URL of the distilled page is not necessarily the URL of the Reading
List entry (in case of redirection).
Store the URL of the page that was distilled in the database.
This CL adds the field in ReadingListEntry.
This field will be used in a next CL.

BUG= 682666 

Review-Url: https://codereview.chromium.org/2647763005
Cr-Commit-Position: refs/heads/master@{#445063}
(cherry picked from commit 695d0c65669b7af00f8daec96d497519c0e0cb46)

Review-Url: https://codereview.chromium.org/2648293002 .
Cr-Commit-Position: refs/branch-heads/2987@{#21}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/proto/reading_list.proto
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_entry.cc
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_entry.h
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_entry_unittest.cc
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_model.h
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_model_impl.cc
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_model_impl.h
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/components/reading_list/ios/reading_list_model_unittest.mm
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/7a16a5ed6f3cc1d871b550ed0187695c0c6cba2c/ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 23 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b

commit f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Mon Jan 23 15:23:23 2017

Store distilled URL during distillation in Reading List on iOS

The URL of the distilled page is not necessarily the URL of the Reading
List entry (incase of redirection).
If the distillation is done on a redirected page, store it along the
distilled_path.

I also filed crbug.com/682705 to have the "redirected_url" directly in the proto file
and avoid having to take it via callback.

BUG= 682666 

Review-Url: https://codereview.chromium.org/2646993002
Cr-Commit-Position: refs/heads/master@{#445071}
(cherry picked from commit 69e7e3b844f1cedf53701f07bf0d3219fc2d3176)

Review-Url: https://codereview.chromium.org/2650593003 .
Cr-Commit-Position: refs/branch-heads/2987@{#22}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/components/dom_distiller/ios/distiller_page_ios.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/dom_distiller/distiller_viewer.cc
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/dom_distiller/distiller_viewer.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_distiller_page.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/reading_list_download_service.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/url_downloader.cc
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/url_downloader.h
[modify] https://crrev.com/f0b185bb6ab2b21c8f23e66b86d02ac73dbbca0b/ios/chrome/browser/reading_list/url_downloader_unittest.mm

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 23 2017

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

commit a86f2882f704d63752ad43dd56950f41e54b9a46
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Mon Jan 23 15:28:21 2017

[Reading List iOS] Display distilled URL instead of entry URL in omnibox

When displaying an offline page, the URL must be the actual distilled
URL and not the Reading List entry URL (which can be a short URL like
goo.gl/...).

BUG= 682666 

Review-Url: https://codereview.chromium.org/2644293002
Cr-Commit-Position: refs/heads/master@{#445077}
(cherry picked from commit 457f401daa5e0667546ab2a96afea821c6e5d88f)

Review-Url: https://codereview.chromium.org/2651693002 .
Cr-Commit-Position: refs/branch-heads/2987@{#24}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/reading_list/offline_url_utils.cc
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/reading_list/offline_url_utils.h
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/reading_list/offline_url_utils_unittest.cc
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/ui/reading_list/offline_page_native_content.mm
[modify] https://crrev.com/a86f2882f704d63752ad43dd56950f41e54b9a46/ios/chrome/browser/ui/reading_list/offline_page_native_content_unittest.mm

Status: Verified (was: Fixed)
Verified in 58.0.2998.0 canary, iPhone 6+ iOS 10.2

goo.gl/6kK6uv This URL is shown as:
https://fr.wikipedia.org/wiki/Google  both with wifi ON/OFF in onmibox and in the Reading List

Verified the issue on latest beta build 57.0.2987.18 on iPhone7+(iOS 10).
goo.gl/6kK6uv This URL is shown as:
https://fr.wikipedia.org/wiki/Google  both with wifi ON/OFF in omnibox and in the Reading List
Components: UI>Browser>ReaderMode

Sign in to add a comment