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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9316086: Fix JavaScript errors in options2 (Closed)

Created:
8 years, 10 months ago by Tyler Breisacher (Chromium)
Modified:
8 years, 10 months ago
Reviewers:
James Hawkins
CC:
chromium-reviews, tfarina, arv (Not doing code reviews)
Visibility:
Public.

Description

Fix JavaScript style errors in options2 BUG= TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=121194

Patch Set 1 #

Total comments: 14

Patch Set 2 : fixes #

Patch Set 3 : revert the "whatsup with that?" change #

Patch Set 4 : use list join #

Total comments: 4

Patch Set 5 : nits #

Patch Set 6 : rebase #

Patch Set 7 : new violations found after rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -162 lines) Patch
M chrome/browser/resources/options2/advanced_options.js View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/options2/autofill_edit_address_overlay.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/autofill_options_list.js View 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/resources/options2/browser_options.js View 1 2 3 4 5 6 4 chunks +10 lines, -12 lines 0 comments Download
M chrome/browser/resources/options2/browser_options_profile_list.js View 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/resources/options2/browser_options_startup_page_list.js View 6 chunks +16 lines, -16 lines 0 comments Download
M chrome/browser/resources/options2/certificate_edit_ca_trust_overlay.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/certificate_import_error_overlay.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/certificate_manager.js View 1 2 3 4 5 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/resources/options2/certificate_tree.js View 1 2 3 4 3 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/resources/options2/content_settings.js View 1 2 3 4 5 6 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/resources/options2/content_settings_exceptions_area.js View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/options2/controlled_setting.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/cookies_list.js View 1 chunk +24 lines, -24 lines 0 comments Download
M chrome/browser/resources/options2/cookies_view.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/handler_options.js View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/options2/handler_options_list.js View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/resources/options2/import_data_overlay.js View 4 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/resources/options2/inline_editable_list.js View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/intents_list.js View 1 2 3 3 chunks +3 lines, -9 lines 0 comments Download
M chrome/browser/resources/options2/language_list.js View 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/resources/options2/language_options.js View 1 2 3 4 5 6 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/resources/options2/manage_profile_overlay.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/options2/options.js View 1 2 3 4 5 2 chunks +8 lines, -1 line 0 comments Download
M chrome/browser/resources/options2/options_page.js View 1 2 3 4 5 7 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/resources/options2/password_manager.js View 1 2 3 4 5 1 chunk +6 lines, -8 lines 0 comments Download
M chrome/browser/resources/options2/password_manager_list.js View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/pref_ui.js View 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/resources/options2/search_engine_manager.js View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/options2/search_engine_manager_engine_list.js View 4 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/resources/options2/search_page.js View 6 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Tyler Breisacher (Chromium)
I know this looks kind of huge but most of the changes are tiny things ...
8 years, 10 months ago (2012-02-03 01:23:17 UTC) #1
James Hawkins
https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js#newcode533 chrome/browser/resources/options2/browser_options.js:533: * }. Whatsup with this change? https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/clear_browser_data_overlay.js File chrome/browser/resources/options2/clear_browser_data_overlay.js ...
8 years, 10 months ago (2012-02-03 01:28:24 UTC) #2
Tyler Breisacher (Chromium)
https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js#newcode533 chrome/browser/resources/options2/browser_options.js:533: * }. The description is a sentence so the ...
8 years, 10 months ago (2012-02-03 01:40:01 UTC) #3
James Hawkins
https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js File chrome/browser/resources/options2/browser_options.js (right): https://chromiumcodereview.appspot.com/9316086/diff/1/chrome/browser/resources/options2/browser_options.js#newcode533 chrome/browser/resources/options2/browser_options.js:533: * }. On 2012/02/03 01:40:01, Tyler Breisacher wrote: > ...
8 years, 10 months ago (2012-02-03 01:41:57 UTC) #4
Tyler Breisacher (Chromium)
I don't think it's a particularly important rule. Let's just turn it off. I'm thinking ...
8 years, 10 months ago (2012-02-03 02:00:31 UTC) #5
James Hawkins
sg
8 years, 10 months ago (2012-02-03 02:00:55 UTC) #6
Tyler Breisacher (Chromium)
It looks like https://chromiumcodereview.appspot.com/9288045/ is getting close to being ready to land but it would ...
8 years, 10 months ago (2012-02-08 20:07:21 UTC) #7
James Hawkins
LGTM with two nits. https://chromiumcodereview.appspot.com/9316086/diff/13001/chrome/browser/resources/options2/certificate_tree.js File chrome/browser/resources/options2/certificate_tree.js (right): https://chromiumcodereview.appspot.com/9316086/diff/13001/chrome/browser/resources/options2/certificate_tree.js#newcode93 chrome/browser/resources/options2/certificate_tree.js:93: delete this.treeLookup_[id]; This should be ...
8 years, 10 months ago (2012-02-09 02:51:52 UTC) #8
Tyler Breisacher (Chromium)
https://chromiumcodereview.appspot.com/9316086/diff/13001/chrome/browser/resources/options2/certificate_tree.js File chrome/browser/resources/options2/certificate_tree.js (right): https://chromiumcodereview.appspot.com/9316086/diff/13001/chrome/browser/resources/options2/certificate_tree.js#newcode93 chrome/browser/resources/options2/certificate_tree.js:93: delete this.treeLookup_[id]; On 2012/02/09 02:51:52, James Hawkins wrote: > ...
8 years, 10 months ago (2012-02-09 03:04:34 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tbreisacher@chromium.org/9316086/13002
8 years, 10 months ago (2012-02-09 03:04:48 UTC) #10
commit-bot: I haz the power
Can't apply patch for file chrome/browser/resources/options2/browser_options.js. While running patch -p1 --forward --force; patching file chrome/browser/resources/options2/browser_options.js ...
8 years, 10 months ago (2012-02-09 03:05:00 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tbreisacher@chromium.org/9316086/13009
8 years, 10 months ago (2012-02-09 03:16:29 UTC) #12
commit-bot: I haz the power
8 years, 10 months ago (2012-02-09 05:07:55 UTC) #13 
Change committed as 121194

Powered by Google App Engine
This is Rietveld 408576698