website_settings: Fix Website Settings popup for chrome://bookmarks.

website_settings: Fix Website Settings popup for chrome://bookmarks.

BUG=151575
TEST=1- go to chrome://bookmarks
     2- click on the icon next to the URL
     3- The bubble should say "You are viewing a secure Google Chrome page."
R=markusheintz@chromium.org
TBR=rsesek@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=158429

[markusheintz_, 2012-09-24 18:36:21]

Thanks a lot for fixing this issue !!!

http://codereview.chromium.org/10973013/diff/1/chrome/browser/ui/website_sett...
File chrome/browser/ui/website_settings/website_settings_utils.cc (right):

http://codereview.chromium.org/10973013/diff/1/chrome/browser/ui/website_sett...
chrome/browser/ui/website_settings/website_settings_utils.cc:16: url.host() ==
extension_misc::kBookmarkManagerId);
I think we only need to test for url.SchemeIs(chrome::kExtensionScheme).

I think the website settings UI should not be displayed for  any URL starting
with the scheme "chrome-extension".

Also this method us unfortunatly not used by Mac (see
chrome/browser/ui/cocoa/website_settings_controller.mm

But I think I can fix this in a separate CL.

[tfarina, 2012-09-24 19:19:03]

http://codereview.chromium.org/10973013/diff/1/chrome/browser/ui/website_sett...
File chrome/browser/ui/website_settings/website_settings_utils.cc (right):

http://codereview.chromium.org/10973013/diff/1/chrome/browser/ui/website_sett...
chrome/browser/ui/website_settings/website_settings_utils.cc:16: url.host() ==
extension_misc::kBookmarkManagerId);
On 2012/09/24 18:36:21, markusheintz_ wrote:
> I think we only need to test for url.SchemeIs(chrome::kExtensionScheme).
> 
> I think the website settings UI should not be displayed for  any URL starting
> with the scheme "chrome-extension".
> 
> Also this method us unfortunatly not used by Mac (see
> chrome/browser/ui/cocoa/website_settings_controller.mm
> 
> But I think I can fix this in a separate CL.

Done.

http://codereview.chromium.org/10973013/diff/10001/chrome/browser/ui/cocoa/we...
File chrome/browser/ui/cocoa/website_settings_bubble_controller.mm (right):

http://codereview.chromium.org/10973013/diff/10001/chrome/browser/ui/cocoa/we...
chrome/browser/ui/cocoa/website_settings_bubble_controller.mm:1140: bool
is_internal_page = url.SchemeIs(chrome::kChromeInternalScheme) ||
Why InternalChromePage is not used here?

[markusheintz_, 2012-09-24 19:19:05]

LGTM. Thanks a lot!

[markusheintz_, 2012-09-24 19:21:02]

http://codereview.chromium.org/10973013/diff/10001/chrome/browser/ui/cocoa/we...
File chrome/browser/ui/cocoa/website_settings_bubble_controller.mm (right):

http://codereview.chromium.org/10973013/diff/10001/chrome/browser/ui/cocoa/we...
chrome/browser/ui/cocoa/website_settings_bubble_controller.mm:1140: bool
is_internal_page = url.SchemeIs(chrome::kChromeInternalScheme) ||
On 2012/09/24 19:19:03, tfarina wrote:
> Why InternalChromePage is not used here?

InternalChromePage should be used here as well. Please use it. Thanks!