Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 22938005: Add ErrorConsole UI for Extension Install Warnings (Closed)

Created:
5 years, 11 months ago by Devlin
Modified:
5 years, 10 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, arv+watch_chromium.org, extensions-reviews_chromium.org, not at google - send to devlin, Dan Beam
Base URL:
https://chromium.googlesource.com/chromium/src.git@dc_ec_install_warnings
Visibility:
Public.

Description

Add Error Console UI for install warnings to the chrome:extensions page (hidden behind the error console switch). Snazzy Images: http://imgur.com/a/7QnMo#0 (updated 9/3) BUG=21734 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221527

Patch Set 1 : #

Patch Set 2 : #

Total comments: 21

Patch Set 3 : Rebase to Master #

Total comments: 20

Patch Set 4 : Yoyo's #

Patch Set 5 : More robust highlighting + testing #

Total comments: 4

Patch Set 6 : Yoz's, Round II #

Total comments: 4

Patch Set 7 : Yoyo's + temporarily remove *.png for apply issue #

Total comments: 79

Patch Set 8 : Dan's #

Total comments: 32

Patch Set 9 : Dan's, round II #

Patch Set 10 : Requested UI Changes #

Total comments: 34

Patch Set 11 : Dan's, III #

Patch Set 12 : #

Patch Set 13 : Rebase to exclude backend #

Total comments: 6

Patch Set 14 : Ben's #

Patch Set 15 : Latest Master (-binary data) #

Patch Set 16 : License #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1097 lines, -43 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +12 lines, -0 lines 0 comments Download
M chrome/browser/extensions/error_console/error_console.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error.css View 1 2 3 4 5 6 7 8 9 10 1 chunk +54 lines, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +17 lines, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +207 lines, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error_overlay.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +12 lines, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error_overlay.html View 1 2 3 4 5 6 7 1 chunk +17 lines, -0 lines 0 comments Download
A chrome/browser/resources/extensions/extension_error_overlay.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +85 lines, -0 lines 0 comments Download
M chrome/browser/resources/extensions/extension_list.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +11 lines, -15 lines 0 comments Download
M chrome/browser/resources/extensions/extensions.html View 1 2 3 4 5 6 7 8 9 10 4 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/resources/extensions/extensions.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +5 lines, -5 lines 0 comments Download
A chrome/browser/ui/webui/extensions/extension_error_handler.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +64 lines, -0 lines 0 comments Download
A chrome/browser/ui/webui/extensions/extension_error_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +174 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +21 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extensions_ui.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_browser_ui.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M extensions/browser/extension_error.h View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +23 lines, -0 lines 0 comments Download
M extensions/browser/extension_error.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +40 lines, -20 lines 0 comments Download
A extensions/browser/manifest_highlighter.h View 1 2 3 4 5 6 7 8 1 chunk +74 lines, -0 lines 0 comments Download
A extensions/browser/manifest_highlighter.cc View 1 2 3 4 5 6 1 chunk +166 lines, -0 lines 0 comments Download
A extensions/browser/manifest_highlighter_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +98 lines, -0 lines 0 comments Download

Messages

Total messages: 37 (0 generated)
Devlin
5 years, 11 months ago (2013-08-14 22:32:42 UTC) #1
Yoyo Zhou
https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc File chrome/browser/extensions/error_console/error_console.cc (right): https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc#newcode183 chrome/browser/extensions/error_console/error_console.cc:183: ReportError(scoped_ptr<const ExtensionError>(new ManifestParsingError( Seeing this makes me wonder whether ...
5 years, 11 months ago (2013-08-16 00:17:25 UTC) #2
Devlin
https://codereview.chromium.org/22938005/diff/57001/chrome/browser/extensions/error_console/error_console_browsertest.cc File chrome/browser/extensions/error_console/error_console_browsertest.cc (right): https://codereview.chromium.org/22938005/diff/57001/chrome/browser/extensions/error_console/error_console_browsertest.cc#newcode39 chrome/browser/extensions/error_console/error_console_browsertest.cc:39: // Verify that all properites of a given |error| ...
5 years, 11 months ago (2013-08-16 18:07:49 UTC) #3
Devlin
https://codereview.chromium.org/22938005/diff/57001/chrome/browser/extensions/error_console/error_console_browsertest.cc File chrome/browser/extensions/error_console/error_console_browsertest.cc (right): https://codereview.chromium.org/22938005/diff/57001/chrome/browser/extensions/error_console/error_console_browsertest.cc#newcode39 chrome/browser/extensions/error_console/error_console_browsertest.cc:39: // Verify that all properites of a given |error| ...
5 years, 11 months ago (2013-08-16 18:07:49 UTC) #4
Yoyo Zhou
LGTM
5 years, 11 months ago (2013-08-16 18:48:59 UTC) #5
Devlin
https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc File chrome/browser/extensions/error_console/error_console.cc (right): https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc#newcode183 chrome/browser/extensions/error_console/error_console.cc:183: ReportError(scoped_ptr<const ExtensionError>(new ManifestParsingError( On 2013/08/16 00:17:25, Yoyo Zhou wrote: ...
5 years, 11 months ago (2013-08-16 23:56:54 UTC) #6
Yoyo Zhou
https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc File chrome/browser/extensions/error_console/error_console.cc (right): https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc#newcode183 chrome/browser/extensions/error_console/error_console.cc:183: ReportError(scoped_ptr<const ExtensionError>(new ManifestParsingError( On 2013/08/16 23:56:54, D Cronin wrote: ...
5 years, 11 months ago (2013-08-19 20:39:45 UTC) #7
Devlin
https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc File chrome/browser/extensions/error_console/error_console.cc (right): https://codereview.chromium.org/22938005/diff/50001/chrome/browser/extensions/error_console/error_console.cc#newcode183 chrome/browser/extensions/error_console/error_console.cc:183: ReportError(scoped_ptr<const ExtensionError>(new ManifestParsingError( On 2013/08/19 20:39:46, Yoyo Zhou wrote: ...
5 years, 11 months ago (2013-08-19 21:42:39 UTC) #8
Yoyo Zhou
LGTM https://codereview.chromium.org/22938005/diff/93001/extensions/browser/manifest_highlighter.cc File extensions/browser/manifest_highlighter.cc (right): https://codereview.chromium.org/22938005/diff/93001/extensions/browser/manifest_highlighter.cc#newcode23 extensions/browser/manifest_highlighter.cc:23: return index < string.size() ? index : std::string::npos; ...
5 years, 11 months ago (2013-08-19 21:48:51 UTC) #9
Devlin
https://codereview.chromium.org/22938005/diff/93001/extensions/browser/manifest_highlighter.cc File extensions/browser/manifest_highlighter.cc (right): https://codereview.chromium.org/22938005/diff/93001/extensions/browser/manifest_highlighter.cc#newcode23 extensions/browser/manifest_highlighter.cc:23: return index < string.size() ? index : std::string::npos; On ...
5 years, 11 months ago (2013-08-20 20:38:22 UTC) #10
Devlin
+ben for c/b/r/extensions
5 years, 11 months ago (2013-08-20 20:39:28 UTC) #11
not at google - send to devlin
I'm happy to take this review but I also don't know how I ended up ...
5 years, 10 months ago (2013-08-20 20:46:12 UTC) #12
Dan Beam
https://codereview.chromium.org/22938005/diff/100002/chrome/browser/resources/extensions/extension_error.css File chrome/browser/resources/extensions/extension_error.css (right): https://codereview.chromium.org/22938005/diff/100002/chrome/browser/resources/extensions/extension_error.css#newcode25 chrome/browser/resources/extensions/extension_error.css:25: .extension-error-simple-wrapper >:first-child { nit: .extension-error-simple-wrapper > :first-child { https://codereview.chromium.org/22938005/diff/100002/chrome/browser/resources/extensions/extension_error.html ...
5 years, 10 months ago (2013-08-20 21:39:25 UTC) #13
Devlin
All addressed. Thanks for the very thorough review, and sorry my JS is so rusty. ...
5 years, 10 months ago (2013-08-20 23:06:51 UTC) #14
Dan Beam
very very close https://chromiumcodereview.appspot.com/22938005/diff/100002/chrome/browser/resources/extensions/extension_error_overlay.html File chrome/browser/resources/extensions/extension_error_overlay.html (right): https://chromiumcodereview.appspot.com/22938005/diff/100002/chrome/browser/resources/extensions/extension_error_overlay.html#newcode6 chrome/browser/resources/extensions/extension_error_overlay.html:6: <div id="extensionErrorOverlay" class="page"> On 2013/08/20 23:06:51, ...
5 years, 10 months ago (2013-08-21 23:41:03 UTC) #15
Devlin
https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css File chrome/browser/resources/extensions/extension_error.css (right): https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css#newcode6 chrome/browser/resources/extensions/extension_error.css:6: cursor: pointer; On 2013/08/21 23:41:03, Dan Beam wrote: > ...
5 years, 10 months ago (2013-08-22 18:29:05 UTC) #16
Devlin
On 2013/08/22 18:29:05, D Cronin wrote: > https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css > File chrome/browser/resources/extensions/extension_error.css (right): > > https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css#newcode6 ...
5 years, 10 months ago (2013-08-23 19:32:15 UTC) #17
Cris Neckar
Remind me please where the entry point to this from the IPC message is.
5 years, 10 months ago (2013-08-23 20:01:53 UTC) #18
Devlin
On 2013/08/23 20:01:53, Cris Neckar wrote: > Remind me please where the entry point to ...
5 years, 10 months ago (2013-08-23 20:04:13 UTC) #19
Devlin
dbeam: friendly ping :)
5 years, 10 months ago (2013-08-26 17:32:20 UTC) #20
Dan Beam
https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css File chrome/browser/resources/extensions/extension_error.css (right): https://codereview.chromium.org/22938005/diff/93002/chrome/browser/resources/extensions/extension_error.css#newcode6 chrome/browser/resources/extensions/extension_error.css:6: cursor: pointer; On 2013/08/22 18:29:06, D Cronin wrote: > ...
5 years, 10 months ago (2013-08-29 00:24:29 UTC) #21
Devlin
https://codereview.chromium.org/22938005/diff/141001/chrome/browser/resources/extensions/extension_error.css File chrome/browser/resources/extensions/extension_error.css (right): https://codereview.chromium.org/22938005/diff/141001/chrome/browser/resources/extensions/extension_error.css#newcode38 chrome/browser/resources/extensions/extension_error.css:38: .extension-error-warn .extension-error-message { On 2013/08/29 00:24:30, Dan Beam (OOO ...
5 years, 10 months ago (2013-08-29 21:02:04 UTC) #22
Devlin
+ finnur for c/b/ui/webui/extensions, c/b/r/extensions Finnur, Dan left for vacation before he could finish the ...
5 years, 10 months ago (2013-09-03 22:23:13 UTC) #23
not at google - send to devlin
lgtm on behalf of danbeam who is OOO, I didn't really go over it in ...
5 years, 10 months ago (2013-09-03 22:51:46 UTC) #24
Devlin
https://codereview.chromium.org/22938005/diff/173001/extensions/browser/extension_error.cc File extensions/browser/extension_error.cc (right): https://codereview.chromium.org/22938005/diff/173001/extensions/browser/extension_error.cc#newcode28 extensions/browser/extension_error.cc:28: return false; On 2013/09/03 22:51:46, kalman wrote: > this ...
5 years, 10 months ago (2013-09-03 23:54:21 UTC) #25
Finnur
Looks like you've got a green light from Dan via Ben. :)
5 years, 10 months ago (2013-09-04 12:01:20 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/22938005/188001
5 years, 10 months ago (2013-09-04 21:15:37 UTC) #27
Devlin
On 2013/09/04 12:01:20, Finnur wrote: > Looks like you've got a green light from Dan ...
5 years, 10 months ago (2013-09-04 21:20:34 UTC) #28
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=23779
5 years, 10 months ago (2013-09-04 21:37:26 UTC) #29
Finnur
OWNERS LGTM stamp on c/b/ui/webui/e
5 years, 10 months ago (2013-09-04 21:38:22 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/22938005/188001
5 years, 10 months ago (2013-09-04 22:25:26 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/22938005/214001
5 years, 10 months ago (2013-09-04 22:37:41 UTC) #32
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
5 years, 10 months ago (2013-09-04 23:11:04 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/22938005/214001
5 years, 10 months ago (2013-09-05 00:10:25 UTC) #34
commit-bot: I haz the power
Retried try job too often on ios_dbg_simulator for step(s) ui_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_simulator&number=82857
5 years, 10 months ago (2013-09-05 01:19:53 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/22938005/214001
5 years, 10 months ago (2013-09-05 21:53:41 UTC) #36
commit-bot: I haz the power
5 years, 10 months ago (2013-09-05 21:56:27 UTC) #37
Message was sent while issue was closed.
Change committed as 221527

Powered by Google App Engine
This is Rietveld 408576698