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

Issue 989133003: Revert of [Fallback icons] Change "explicit flow" interface so color hex strings don't use "#". (Closed)

Created:
5 years, 9 months ago by Bence
Modified:
5 years, 9 months ago
Reviewers:
James Hawkins, huangs
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Fallback icons] Change "explicit flow" interface so color hex strings don't use "#". (patchset #4 id:60001 of https://codereview.chromium.org/988863002/) Reason for revert: This seems to be causing FallbackIconUrlParserTest unit_tests to fail, see, for example, https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/7625 https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/7297 https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29/builds/39924 https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/10797 Original issue's description: > [Fallback icons] Change "explicit flow" interface so color hex strings don't use "#". > > Design: go/chrome-fallback-icons > > Fallback icon "explicit flow" URL used to have '#' in hex colors, e.g.: > > chrome://fallback-icon/,#01f,#123456,,/http://www.google.com > > Problem: "#" denotes fragment in URLs, so we get rid of it. This causes no ambiguity > with named color (e.g., "red") because no named color consists of letters a-f only. > Also adding "ARGB" hex color since this is supported in Skia color parsing. > > BUG=455063 > > Committed: https://crrev.com/8fa91aabefbe9784eca0c169bc2ad2dd92f5d887 > Cr-Commit-Position: refs/heads/master@{#319630} TBR=jhawkins@chromium.org,huangs@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=455063 Committed: https://crrev.com/8d63f0cd7fdd28dd64b9c25a0c4044bac88df5a5 Cr-Commit-Position: refs/heads/master@{#319677}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -88 lines) Patch
M chrome/browser/ui/webui/fallback_icon_source.h View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/common/favicon/fallback_icon_url_parser.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common/favicon/fallback_icon_url_parser.cc View 2 chunks +3 lines, -34 lines 0 comments Download
M chrome/common/favicon/fallback_icon_url_parser_unittest.cc View 11 chunks +48 lines, -48 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Bence
Created Revert of [Fallback icons] Change "explicit flow" interface so color hex strings don't use ...
5 years, 9 months ago (2015-03-09 17:45:37 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/989133003/1
5 years, 9 months ago (2015-03-09 17:46:56 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 9 months ago (2015-03-09 17:48:11 UTC) #3
commit-bot: I haz the power
5 years, 9 months ago (2015-03-09 17:49:37 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8d63f0cd7fdd28dd64b9c25a0c4044bac88df5a5
Cr-Commit-Position: refs/heads/master@{#319677}

Powered by Google App Engine
This is Rietveld 408576698