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

Issue 2631393002: Move "Read Later" to the third place in the context menu (Closed)

Created:
3 years, 11 months ago by gambard
Modified:
3 years, 11 months ago
CC:
chromium-reviews, marq+watch_chromium.org, 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

Move "Read Later" to the third place in the context menu BUG=681627 Review-Url: https://codereview.chromium.org/2631393002 Cr-Commit-Position: refs/heads/master@{#444010} Committed: https://chromium.googlesource.com/chromium/src/+/5fd4034984b118b932a891b849320c147f47e702

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -14 lines) Patch
M ios/chrome/browser/ui/browser_view_controller.mm View 2 chunks +14 lines, -14 lines 1 comment Download

Messages

Total messages: 10 (5 generated)
gambard
PTAL, quick review.
3 years, 11 months ago (2017-01-17 08:19:08 UTC) #2
marq (ping after 24h)
lgtm
3 years, 11 months ago (2017-01-17 09:08:42 UTC) #3
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/2631393002/1
3 years, 11 months ago (2017-01-17 09:09:13 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/5fd4034984b118b932a891b849320c147f47e702
3 years, 11 months ago (2017-01-17 09:18:25 UTC) #8
Olivier
3 years, 11 months ago (2017-01-17 09:34:55 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/2631393002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2631393002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:2451: if
(web::UrlHasWebScheme(link)) {
I know this is not in the scope of this CL, but this test is too permissive for
ReadingList.
It should be SchemeIsHTTPOrHTTPS.

Powered by Google App Engine
This is Rietveld 408576698