Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2506473002:
Page Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center). (Closed)
DescriptionPage Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center).
Since "Learn more" should show up unconditionally, this CL also removes the
include_details_label_link calculation/parameter.
BUG=646465
Committed: https://crrev.com/b822f27141186d301f3ee03805d29ecd9d00bbf7
Cr-Commit-Position: refs/heads/master@{#435083}
Patch Set 1 #Patch Set 2 : Update comment. #
Total comments: 12
Patch Set 3 : Address nits. #Patch Set 4 : Views Page Info: Change "Details" link (sec. panel) to "Learn more" (help center). #Patch Set 5 : Rebase. #Messages
Total messages: 18 (11 generated)
lgarron@chromium.org changed reviewers: + msw@chromium.org
msw@, could you review?
Description was changed from ========== Views Page Info: Change "Details" link (sec. panel) to "Learn more" (help center). Since "Learn more" should show up unconditionally, this CL also removes the include_details_label_link calculation/parameter. BUG=646465 ========== to ========== Page Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center). Since "Learn more" should show up unconditionally, this CL also removes the include_details_label_link calculation/parameter. BUG=646465 ==========
The CQ bit was checked by lgarron@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc (left): https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:790: WebsiteSettings::WEBSITE_SETTINGS_SECURITY_DETAILS_OPENED); Deprecate this and WEBSITE_SETTINGS_CERTIFICATE_DIALOG_OPENED if they'll be removed on Cocoa too. https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc (right): https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:19: #include "chrome/browser/devtools/devtools_toggle_action.h" nit: remove devtools includes https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:36: #include "components/prefs/pref_service.h" nit: remove prefs includes https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:278: l10n_util::GetStringUTF16(IDS_PAGE_INFO_LEARN_MORE); nit: inline below, nix |learn_more_text| https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:463: is_devtools_disabled_ = nit: remove this. https://codereview.chromium.org/2506473002/diff/20001/components/pageinfo_str... File components/pageinfo_strings.grdp (right): https://codereview.chromium.org/2506473002/diff/20001/components/pageinfo_str... components/pageinfo_strings.grdp:75: <message name="IDS_PAGE_INFO_LEARN_MORE" desc="Text on a button or a link that takes the user to a page explaining connection security details."> Use IDS_LEARN_MORE instead of adding a duplicate string.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:40001) has been deleted
https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc (left): https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:790: WebsiteSettings::WEBSITE_SETTINGS_SECURITY_DETAILS_OPENED); On 2016/11/15 at 01:56:07, msw wrote: > Deprecate this and WEBSITE_SETTINGS_CERTIFICATE_DIALOG_OPENED if they'll be removed on Cocoa too. I've made a separate CL for this: https://codereview.chromium.org/2521173004 https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc (right): https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:19: #include "chrome/browser/devtools/devtools_toggle_action.h" On 2016/11/15 at 01:56:07, msw wrote: > nit: remove devtools includes Done. https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:36: #include "components/prefs/pref_service.h" On 2016/11/15 at 01:56:07, msw wrote: > nit: remove prefs includes Done. https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:278: l10n_util::GetStringUTF16(IDS_PAGE_INFO_LEARN_MORE); On 2016/11/15 at 01:56:07, msw wrote: > nit: inline below, nix |learn_more_text| Done. https://codereview.chromium.org/2506473002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:463: is_devtools_disabled_ = On 2016/11/15 01:56:07, msw wrote: > nit: remove this. Done. https://codereview.chromium.org/2506473002/diff/20001/components/pageinfo_str... File components/pageinfo_strings.grdp (right): https://codereview.chromium.org/2506473002/diff/20001/components/pageinfo_str... components/pageinfo_strings.grdp:75: <message name="IDS_PAGE_INFO_LEARN_MORE" desc="Text on a button or a link that takes the user to a page explaining connection security details."> On 2016/11/15 at 01:56:07, msw wrote: > Use IDS_LEARN_MORE instead of adding a duplicate string. Ooh, neat. Done.
lgtm
The CQ bit was checked by lgarron@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 90001, "attempt_start_ts": 1480450429877530,
"parent_rev": "05818e4d334f863f0096cb6d5dc430b253e976e3", "commit_rev":
"091d2df05be1ff1c3f56683b1ab0b2090c26cbdb"}
CQ is committing da patch.
Bot data: {"patchset_id": 90001, "attempt_start_ts": 1480450429877530,
"parent_rev": "8cf51abada15c09c0c3863935a1a8c9ec9beba3b", "commit_rev":
"a9fe38b8fce80f0a13117584dbf5c1184e3cb470"}
Message was sent while issue was closed.
Committed patchset #5 (id:90001)
Message was sent while issue was closed.
Description was changed from ========== Page Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center). Since "Learn more" should show up unconditionally, this CL also removes the include_details_label_link calculation/parameter. BUG=646465 ========== to ========== Page Info (Views): Change "Details" link (sec. panel) to "Learn more" (help center). Since "Learn more" should show up unconditionally, this CL also removes the include_details_label_link calculation/parameter. BUG=646465 Committed: https://crrev.com/b822f27141186d301f3ee03805d29ecd9d00bbf7 Cr-Commit-Position: refs/heads/master@{#435083} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/b822f27141186d301f3ee03805d29ecd9d00bbf7 Cr-Commit-Position: refs/heads/master@{#435083} |
