Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(90)

Issue 2644293002: [Reading List iOS] Display distilled URL instead of entry URL in omnibox (Closed)

Created:
3 years, 11 months ago by Olivier
Modified:
3 years, 11 months ago
Reviewers:
gambard, jif
CC:
chromium-reviews, marq+watch_chromium.org, stkhapugin, pkl (ping after 24h if needed), noyau+watch_chromium.org, sdefresne+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[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} Committed: https://chromium.googlesource.com/chromium/src/+/457f401daa5e0667546ab2a96afea821c6e5d88f

Patch Set 1 #

Total comments: 26

Patch Set 2 : feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+154 lines, -48 lines) Patch
M ios/chrome/browser/reading_list/offline_url_utils.h View 1 1 chunk +15 lines, -5 lines 0 comments Download
M ios/chrome/browser/reading_list/offline_url_utils.cc View 3 chunks +23 lines, -5 lines 0 comments Download
M ios/chrome/browser/reading_list/offline_url_utils_unittest.cc View 1 1 chunk +56 lines, -15 lines 0 comments Download
M ios/chrome/browser/reading_list/reading_list_web_state_observer.mm View 1 chunk +2 lines, -2 lines 0 comments Download
M ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm View 1 3 chunks +4 lines, -6 lines 0 comments Download
M ios/chrome/browser/ui/reading_list/offline_page_native_content.mm View 5 chunks +14 lines, -9 lines 0 comments Download
M ios/chrome/browser/ui/reading_list/offline_page_native_content_unittest.mm View 1 4 chunks +40 lines, -6 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 11 (5 generated)
Olivier
3 years, 11 months ago (2017-01-20 09:13:39 UTC) #2
gambard
https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h File ios/chrome/browser/reading_list/offline_url_utils.h (right): https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h#newcode19 ios/chrome/browser/reading_list/offline_url_utils.h:19: GURL OfflineURLForPath(const base::FilePath& distilled_path, Can you change the function ...
3 years, 11 months ago (2017-01-20 14:49:35 UTC) #3
jif
lgtm https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h File ios/chrome/browser/reading_list/offline_url_utils.h (right): https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h#newcode19 ios/chrome/browser/reading_list/offline_url_utils.h:19: GURL OfflineURLForPath(const base::FilePath& distilled_path, Consider explaining why |entry_url| ...
3 years, 11 months ago (2017-01-20 15:01:25 UTC) #4
Olivier
Thanks https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h File ios/chrome/browser/reading_list/offline_url_utils.h (right): https://codereview.chromium.org/2644293002/diff/1/ios/chrome/browser/reading_list/offline_url_utils.h#newcode19 ios/chrome/browser/reading_list/offline_url_utils.h:19: GURL OfflineURLForPath(const base::FilePath& distilled_path, On 2017/01/20 15:01:25, jif ...
3 years, 11 months ago (2017-01-20 16:09:15 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2644293002/20001
3 years, 11 months ago (2017-01-20 16:43:38 UTC) #8
commit-bot: I haz the power
3 years, 11 months ago (2017-01-20 16:54:03 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/457f401daa5e0667546ab2a96afe...

Powered by Google App Engine
This is Rietveld 408576698