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

Issue 1137353004: Add a separate param for https scheme hightlight to emphasizeUrl (Closed)

Created:
5 years, 7 months ago by Yusuf
Modified:
5 years, 7 months ago
Reviewers:
Ted C
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a separate param for https scheme hightlight to emphasizeUrl Currently the utility call for emphasizing urls uses useDarkText as a signal to decide whether https scheme should be colored. Since with theme colors this logic gets more compicated, this change adds another explicit param to control the coloring of https. BUG=482780 Committed: https://crrev.com/dc364e9a13403cbc7b7eed3239baa463256b9ca4 Cr-Commit-Position: refs/heads/master@{#330183}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Renamed param and removed security state check to caller #

Patch Set 3 : Assert that emphasizeHttps doesn't violate security state concerns #

Patch Set 4 : Fix tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -20 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizer.java View 1 2 2 chunks +8 lines, -5 lines 0 comments Download
M chrome/android/javatests_shell/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizerTest.java View 1 2 3 9 chunks +14 lines, -14 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
Yusuf
5 years, 7 months ago (2015-05-14 20:43:14 UTC) #2
Ted C
https://codereview.chromium.org/1137353004/diff/1/chrome/android/java/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizer.java File chrome/android/java/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizer.java (right): https://codereview.chromium.org/1137353004/diff/1/chrome/android/java/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizer.java#newcode143 chrome/android/java/src/org/chromium/chrome/browser/omnibox/OmniboxUrlEmphasizer.java:143: * @param useLightSecurityTheme Whether the https scheme should not ...
5 years, 7 months ago (2015-05-14 21:06:51 UTC) #3
Ted C
lgtm
5 years, 7 months ago (2015-05-14 21:28:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1137353004/40001
5 years, 7 months ago (2015-05-15 19:01:52 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: android_compile_dbg on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile_dbg/builds/17029) linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, ...
5 years, 7 months ago (2015-05-15 19:29:51 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1137353004/60001
5 years, 7 months ago (2015-05-15 19:46:45 UTC) #12
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 7 months ago (2015-05-15 20:17:15 UTC) #13
325583992843-0clp48nf3nc7pk3ll9lp3k65k4ra53j4
Patchset 4 (id:??) landed as https://crrev.com/dc364e9a13403cbc7b7eed3239baa463256b9ca4 Cr-Commit-Position: refs/heads/master@{#330183}
5 years, 7 months ago (2015-05-16 05:29:50 UTC) #14
commit-bot: I haz the power
5 years, 7 months ago (2015-05-18 11:26:50 UTC) #15
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/dc364e9a13403cbc7b7eed3239baa463256b9ca4
Cr-Commit-Position: refs/heads/master@{#330183}

Powered by Google App Engine
This is Rietveld 408576698