Don't truncate image in plugins and flags pages

Don't truncate image in plugins and flags pages

BUG=134145
TEST=Check the icons at the top of the chrome://plugins and chrome://flags pages
are not truncated; check that bug 83282 hasn't been re-introduced.


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

[benwells, 2012-06-25 01:36:08]

On 2012/06/25 00:40:30, benwells wrote:

To explain why I chose 40px:
- the text in the heading is placed vertically by the padding of the div
surrounding the heading
- the bitmap's top is placed by the padding of the heading
- where the bitmap got truncated was determined by the padding of the header and
the content height of the header
- the content height of the header depends on the font being used to render it
and the platform.

The 40px I chose is a nice round number and gives lots of breathing space, so if
the content height changes slightly there shouldn't be any truncation of images.
As webkit doesn't jump when searching in overflow hidden text anymore, this
breathing space is fine.

I think what has happened is that the content height has changed slightly, or is
different on different platforms, maybe due to fonts or font rendering changes.
Out of interest I went back to after the original change and rebuilt on Linux,
the flags page looked fine but the plugins page was slightly truncated. I can't
remember what platform I made the change on, I'm guessing Windows.

If this sounds wrong let me know, I don't like stuffing things up and would like
to avoid it in future!

[James Hawkins, 2012-06-25 02:38:58]

LGTM

[Nico, 2012-06-25 02:40:22]

I tried to patch this in to take a look, but it doesn't apply cleanly on head.

On Sun, Jun 24, 2012 at 5:40 PM,  <benwells@chromium.org> wrote:
> Reviewers: James Hawkins,
>
> Description:
> Don't truncate image in plugins and flags pages
>
> BUG=134145
> TEST=Check the icons at the top of the chrome://plugins and chrome://flags
> pages
> are not truncated; check that bug 83282 hasn't been re-introduced.
>
>
> Please review this at http://codereview.chromium.org/10656016/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/resources/flags.css
>  M chrome/browser/resources/plugins.css
>
>
> Index: chrome/browser/resources/flags.css
> diff --git a/chrome/browser/resources/flags.css
> b/chrome/browser/resources/flags.css
> index
>
7a562b4404ef14b12efd95430517dcd61163dfde..d9a6e0cabd255720f203ae68d18cb0791b9ef214
> 100644
> --- a/chrome/browser/resources/flags.css
> +++ b/chrome/browser/resources/flags.css
> @@ -31,7 +31,7 @@ div#header {
>   background: url('../../app/theme/flags_section.png') 0 20px no-repeat;
>   display: inline;
>   margin: 0;
> -  padding-bottom: 20px;
> +  padding-bottom: 40px;
>   padding-top: 40px;
>  }
>
> Index: chrome/browser/resources/plugins.css
> diff --git a/chrome/browser/resources/plugins.css
> b/chrome/browser/resources/plugins.css
> index
>
a585fc2bc185b5ecc23d97f9f3673f763a7f8e43..fec96b7f88e4a30f5a1da5444f8102527691928d
> 100644
> --- a/chrome/browser/resources/plugins.css
> +++ b/chrome/browser/resources/plugins.css
> @@ -28,7 +28,7 @@ div#header {
>   background:
> url('../../app/theme/default_100_percent/extensions_section.png') 0 20px
> no-repeat;
>   display: inline;
>   margin: 0;
> -  padding-bottom: 20px;
> +  padding-bottom: 40px;
>   padding-left: 75px;
>   padding-top: 40px;
>  }
>
>

[benwells, 2012-06-25 03:09:37]

+thakis for review

Rebased so should apply cleanly now. Let me know how it looks.

[Nico, 2012-06-25 03:12:39]

lgtm

On Sun, Jun 24, 2012 at 8:09 PM,  <benwells@chromium.org> wrote:
> +thakis for review
>
> Rebased so should apply cleanly now. Let me know how it looks.
>
> http://codereview.chromium.org/10656016/

[commit-bot: I haz the power, 2012-06-25 03:15:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10656016/6001

[commit-bot: I haz the power, 2012-06-25 04:27:24]

Change committed as 143875