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

Issue 2254273003: Remove text encoding UI (Closed)

Created:
4 years, 4 months ago by Jinsuk Kim
Modified:
3 years, 5 months ago
CC:
aelias_OOO_until_Jul13, arv+watch_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dbeam+watch-settings_chromium.org, dbeam+watch-options_chromium.org, dcheng, Dmitry Titov, jam, jennb, jianli, jshin+watch_chromium.org, michaelpg+watch-md-ui_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-options_chromium.org, nasko+codewatch_chromium.org, stevenjb+watch-md-settings_chromium.org, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove text encoding UI Text encoding UI is removed as a part of the project Eraser effort. Manual encoding selection is not necessary any more as the new encoding detector is turned on by default. Request for manual encoding switching which may still exist can be met by utilizing Chrome extension. See the bug for detailed discussion. Also removed the default encoding in Settings -> Customize fonts -> Fonts and encoding. Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb BUG=597488 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/72125e2b16381f98e4bd44ae67240a736a648f5a Cr-Commit-Position: refs/heads/master@{#415497}

Patch Set 1 #

Total comments: 16

Patch Set 2 : comments #

Total comments: 9

Patch Set 3 : comments #

Total comments: 8

Patch Set 4 : comments #

Total comments: 4

Patch Set 5 : removed unused ipc messages #

Patch Set 6 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -1576 lines) Patch
M chrome/app/chrome_command_ids.h View 1 2 3 4 5 2 chunks +0 lines, -42 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 2 3 4 5 4 chunks +9 lines, -21 lines 0 comments Download
M chrome/app/nibs/MainMenu.xib View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/app/resources/locale_settings.grd View 1 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/app/settings_strings.grdp View 1 2 3 4 5 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/app_controller_mac.mm View 2 chunks +0 lines, -11 lines 0 comments Download
M chrome/browser/browser_encoding_browsertest.cc View 1 4 chunks +3 lines, -47 lines 0 comments Download
M chrome/browser/character_encoding.h View 1 2 3 1 chunk +3 lines, -95 lines 0 comments Download
M chrome/browser/character_encoding.cc View 1 2 3 2 chunks +42 lines, -506 lines 0 comments Download
A chrome/browser/character_encoding_unittest.cc View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 4 5 3 chunks +3 lines, -5 lines 0 comments Download
M chrome/browser/prefs/browser_prefs.cc View 1 2 3 4 5 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/font_settings.html View 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/browser/resources/options/font_settings.js View 5 chunks +3 lines, -7 lines 0 comments Download
M chrome/browser/resources/settings/appearance_page/appearance_fonts_page.html View 1 2 3 4 5 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/resources/settings/appearance_page/appearance_fonts_page.js View 3 chunks +1 line, -13 lines 0 comments Download
M chrome/browser/resources/settings/appearance_page/appearance_page.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/browser.h View 1 2 3 chunks +0 lines, -9 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 4 chunks +0 lines, -40 lines 0 comments Download
M chrome/browser/ui/browser_browsertest.cc View 1 2 3 4 5 3 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/browser_command_controller.cc View 1 2 3 4 5 4 chunks +0 lines, -105 lines 0 comments Download
M chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_command_handler.mm View 3 chunks +1 line, -26 lines 0 comments Download
D chrome/browser/ui/cocoa/encoding_menu_controller_delegate_mac.h View 1 chunk +0 lines, -23 lines 0 comments Download
D chrome/browser/ui/cocoa/encoding_menu_controller_delegate_mac.mm View 1 chunk +0 lines, -59 lines 0 comments Download
M chrome/browser/ui/prefs/prefs_tab_helper.cc View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model.h View 2 chunks +0 lines, -22 lines 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model.cc View 1 2 3 4 5 3 chunks +0 lines, -69 lines 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model_unittest.cc View 1 chunk +0 lines, -10 lines 0 comments Download
D chrome/browser/ui/toolbar/encoding_menu_controller.h View 1 chunk +0 lines, -55 lines 0 comments Download
D chrome/browser/ui/toolbar/encoding_menu_controller.cc View 1 chunk +0 lines, -142 lines 0 comments Download
D chrome/browser/ui/toolbar/encoding_menu_controller_unittest.cc View 1 chunk +0 lines, -104 lines 0 comments Download
M chrome/browser/ui/views/frame/system_menu_model_builder.h View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/system_menu_model_builder.cc View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/options/font_settings_handler.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options/font_settings_handler.cc View 5 chunks +3 lines, -37 lines 0 comments Download
M chrome/browser/ui/webui/settings/font_handler.cc View 1 1 chunk +0 lines, -29 lines 0 comments Download
M chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc View 1 2 3 4 5 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/chrome_browser_ui.gypi View 1 2 3 4 5 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 2 chunks +1 line, -1 line 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 5 1 chunk +0 lines, -10 lines 0 comments Download
M content/common/view_messages.h View 1 2 3 4 1 chunk +0 lines, -7 lines 0 comments Download
M content/public/browser/web_contents.h View 1 chunk +0 lines, -11 lines 0 comments Download
M content/renderer/render_view_impl.h View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 5 2 chunks +0 lines, -12 lines 0 comments Download
M tools/metrics/actions/actions.xml View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 62 (33 generated)
Jinsuk Kim
stevenjb@ please review changes in chrome/browser/resources grt@ please review changes in chrome/app msw@ please review ...
4 years, 4 months ago (2016-08-22 02:12:14 UTC) #11
grt (UTC plus 2)
chrome/app rubberstamp lgtm
4 years, 4 months ago (2016-08-22 10:01:59 UTC) #12
stevenjb
lgtm
4 years, 4 months ago (2016-08-22 15:53:39 UTC) #13
msw
It's great to see this cleanup! I have some comments/qs. https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.cc File chrome/browser/character_encoding.cc (left): https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.cc#oldcode40 ...
4 years, 4 months ago (2016-08-22 21:59:54 UTC) #14
Jinsuk Kim
https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.cc File chrome/browser/character_encoding.cc (left): https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.cc#oldcode40 chrome/browser/character_encoding.cc:40: { IDC_ENCODING_UTF8, "UTF-8", IDS_ENCODING_UNICODE }, On 2016/08/22 21:59:53, msw ...
4 years, 4 months ago (2016-08-23 07:09:22 UTC) #15
msw
https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.h File chrome/browser/character_encoding.h (right): https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.h#newcode48 chrome/browser/character_encoding.h:48: // FUNCTION IS NOT THREADSAFE. You must run this ...
4 years, 4 months ago (2016-08-23 18:06:27 UTC) #16
Jinsuk Kim
PTAL. Also added a unit test for GetCanonicalEncodingNameByAliasName. https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.h File chrome/browser/character_encoding.h (right): https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_encoding.h#newcode48 chrome/browser/character_encoding.h:48: // ...
4 years, 4 months ago (2016-08-23 22:46:14 UTC) #17
msw
lgtm with nits, a suggestion for followup, and one potentially important comment (needing to register ...
4 years, 4 months ago (2016-08-23 23:10:55 UTC) #18
Jinsuk Kim
Thanks for reviewing. Please take another look at browser_prefs.cc to make sure the change is ...
4 years, 4 months ago (2016-08-24 08:23:42 UTC) #19
msw
lgtm
4 years, 4 months ago (2016-08-24 16:42:35 UTC) #20
Lei Zhang
lgtm
4 years, 4 months ago (2016-08-24 18:30:29 UTC) #21
Jinsuk Kim
asvitkine@ please review changes in tools/metrics/
4 years, 4 months ago (2016-08-24 19:22:08 UTC) #23
Jinsuk Kim
jwd@ please review changes in tools/metrics/.
4 years, 4 months ago (2016-08-24 19:22:52 UTC) #25
jwd
LGTM with tiny nit. https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/actions.xml#newcode12138 tools/metrics/actions/actions.xml:12138: <obsolete>Obsolete as of 8/2016</obsolete> tiny ...
4 years, 3 months ago (2016-08-25 17:27:39 UTC) #26
Jinsuk Kim
sievers@ would you review changes in content/? I'd like owner's LGTM. Thanks.
4 years, 3 months ago (2016-08-26 22:53:57 UTC) #27
no sievers
https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_contents/web_contents_impl.cc File content/browser/web_contents/web_contents_impl.cc (left): https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_contents/web_contents_impl.cc#oldcode2921 content/browser/web_contents/web_contents_impl.cc:2921: Send(new ViewMsg_ResetPageEncodingToDefault(GetRoutingID())); What about removal of the msgs and ...
4 years, 3 months ago (2016-08-29 23:54:53 UTC) #28
Jinsuk Kim
sievers@ PTAL rickyz@ Would you review changes in content/common/view_messages.h? https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_contents/web_contents_impl.cc File content/browser/web_contents/web_contents_impl.cc (left): https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_contents/web_contents_impl.cc#oldcode2921 content/browser/web_contents/web_contents_impl.cc:2921: ...
4 years, 3 months ago (2016-08-30 01:08:23 UTC) #31
no sievers
content lgtm but it looks like the encoding override code in blink is potentially unused ...
4 years, 3 months ago (2016-08-30 20:39:18 UTC) #46
rickyz (no longer on Chrome)
On 2016/08/30 at 20:39:18, sievers wrote: > content lgtm but it looks like the encoding ...
4 years, 3 months ago (2016-08-30 22:16:33 UTC) #47
Jinsuk Kim
On 2016/08/30 20:39:18, sievers wrote: > content lgtm but it looks like the encoding override ...
4 years, 3 months ago (2016-08-30 22:26:51 UTC) #48
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/2254273003/160001
4 years, 3 months ago (2016-08-30 22:27:46 UTC) #51
commit-bot: I haz the power
Committed patchset #6 (id:160001)
4 years, 3 months ago (2016-08-30 23:52:24 UTC) #53
commit-bot: I haz the power
Patchset 6 (id:??) landed as https://crrev.com/72125e2b16381f98e4bd44ae67240a736a648f5a Cr-Commit-Position: refs/heads/master@{#415497}
4 years, 3 months ago (2016-08-30 23:53:41 UTC) #55
snowflake2396
https://tinyspacesliving.com/best-sleeper-sofas-buying-guide/
3 years, 9 months ago (2017-03-11 08:37:46 UTC) #57
snowflake2396
3 years, 9 months ago (2017-03-11 10:36:36 UTC) #58
snowflake2396
On 2017/03/11 10:36:36, snowflake2396 wrote:
3 years, 9 months ago (2017-03-11 10:37:48 UTC) #59
ikimotho
On 2017/03/11 10:37:48, snowflake2396 wrote: > On 2017/03/11 10:36:36, snowflake2396 wrote: https://bestbuyhq.com/best-american-pressure-canner/ https://bestbuyhq.com/
3 years, 6 months ago (2017-06-23 10:52:22 UTC) #60
ikimotho
On 2017/06/23 10:52:22, ikimotho wrote: > On 2017/03/11 10:37:48, snowflake2396 wrote: > > On 2017/03/11 ...
3 years, 5 months ago (2017-06-26 16:01:05 UTC) #61
ikimotho
3 years, 5 months ago (2017-06-26 16:01:38 UTC) #62
Message was sent while issue was closed.
On 2017/03/11 08:37:46, snowflake2396 wrote:
> https://tinyspacesliving.com/best-sleeper-sofas-buying-guide/

http://floralbeautyspot.com/

Powered by Google App Engine
This is Rietveld 408576698