[Views] Set the width of the Website Settings UI dynamically.

[Views] Set the width of the Website Settings UI dynamically based on the size of the Permissions Tab contents.


BUG=159156
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166748

[markusheintz_, 2012-11-07 16:58:50]

Hey Mike

could you please review this CL? It's fixing a pressing bug. Thanks a lot.

[msw, 2012-11-07 22:43:08]

Should there be a larger max width? It'd be odd to exceed screen size if that's
possible.

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
(right):

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:410: int
width = kMinPopupWidth - kPermissionsSectionPaddingLeft;
nit: replace kMinPopupWidth with a kMinContentWidth that doesn't have this
padding value built-in to simplify.

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:413:
width = site_data_content_->GetPreferredSize().width();
nit: nix the width comparison and do std::max here and below.

[markusheintz_, 2012-11-08 16:38:47]

The bubble width is chosen in a way that no lines on the Permissions tab are cut
off due to localizing the UI. So the bubble width is limited by the language
with the widest text (only in this particular language). So in theory the UI
should not become arbitrary wide.

A max width would be an arbitrary value that I works for all languages, let's
say 500px. In both cases (with and without the max width) the UI is very likely
broken due to a bug if it's width is >= 500px (bad string or bad string and
cutoff rows). With the max value we can only ensure that the brokenness is
predictably wide.

I added a max width of 500. Let me know if you still have any questions or
concerns.

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
File chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
(right):

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:410: int
width = kMinPopupWidth - kPermissionsSectionPaddingLeft;
On 2012/11/07 22:43:08, msw wrote:
> nit: replace kMinPopupWidth with a kMinContentWidth that doesn't have this
> padding value built-in to simplify.

Done.

http://codereview.chromium.org/11362134/diff/6003/chrome/browser/ui/views/web...
chrome/browser/ui/views/website_settings/website_settings_popup_view.cc:413:
width = site_data_content_->GetPreferredSize().width();
On 2012/11/07 22:43:08, msw wrote:
> nit: nix the width comparison and do std::max here and below.

Done.

[msw, 2012-11-08 18:24:30]

LGTM; thanks!

[commit-bot: I haz the power, 2012-11-08 18:31:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/markusheintz@chromium.org/11362134/3003

[commit-bot: I haz the power, 2012-11-08 20:43:50]

Change committed as 166748