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

Issue 523153: Make about:plugin page get rendered properly in Hebrew and Arabic Chrome.... (Closed)

Created:
10 years, 11 months ago by jungshik at Google
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com, ben+cc_chromium.org
Visibility:
Public.

Description

Make about:plugin page get rendered properly in Hebrew and Arabic Chrome. Also make it use the same font as other 'domUI' pages. While doing so, get rid of some unnecessary string conversions in browser_about_handler.cc BUG=31782 TEST=Run Chrome with --lang=ar / --lang=he on Windows (or LANGUAGE=ar or LANGUAGE=he on Linux) and go to 'about:plugins' page. The page should be properly 'RTLized'. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35851

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -25 lines) Patch
M chrome/browser/browser_about_handler.cc View 3 chunks +11 lines, -16 lines 1 comment Download
M chrome/browser/resources/about_plugins.html View 4 chunks +19 lines, -9 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
jungshik at Google
10 years, 11 months ago (2010-01-08 00:10:36 UTC) #1
arv (Not doing code reviews)
http://codereview.chromium.org/523153/diff/1/2 File chrome/browser/browser_about_handler.cc (right): http://codereview.chromium.org/523153/diff/1/2#newcode536 chrome/browser/browser_about_handler.cc:536: return jstemplate_builder::GetTemplatesHtml( Actually, I would prefer if we kept ...
10 years, 11 months ago (2010-01-08 00:25:48 UTC) #2
jungshik at Google
On 2010/01/08 00:25:48, arv wrote: > http://codereview.chromium.org/523153/diff/1/2 > File chrome/browser/browser_about_handler.cc (right): > > http://codereview.chromium.org/523153/diff/1/2#newcode536 > ...
10 years, 11 months ago (2010-01-08 01:28:27 UTC) #3
arv (Not doing code reviews)
10 years, 11 months ago (2010-01-08 02:24:35 UTC) #4
LGTM

On 2010/01/08 01:28:27, Jungshik Shin wrote:
> On 2010/01/08 00:25:48, arv wrote:
> > http://codereview.chromium.org/523153/diff/1/2
> > File chrome/browser/browser_about_handler.cc (right):
> > 
> > http://codereview.chromium.org/523153/diff/1/2#newcode536
> > chrome/browser/browser_about_handler.cc:536: return
> > jstemplate_builder::GetTemplatesHtml(
> > Actually, I would prefer if we kept it the old way and added a todo to stop
> > using JSTemplate for about_version.html since it is not needed there. The
> > I18nTemplate is much simpler and faster.
> 
> Perhaps, I know only little about two. 
> 
> The old (current) way is calling GetTemplateHtml (without 's') and obviously
it
> doesn't pick up i18n-values (textdirection and font*.)  Do you mean calling
> GetI18nTemplateHtml?  To do that, I really have to make a switch now, don't I?


I don't think you need to update the HTML files at this point. We should do it
at some point in the future but it is prertty low priority.

>  
> >
> > http://codereview.chromium.org/523153/diff/1/3
> > File chrome/browser/resources/about_plugins.html (right):
> > 
> > http://codereview.chromium.org/523153/diff/1/3#newcode93
> > chrome/browser/resources/about_plugins.html:93: html[dir='ltr'] td + td {
> > It is enough to only do the rtl ones.
> > 
> > th + th,
> > td + td {
> >   border-left: 1px dotted ThreeDShadow;
> > }
> 
> In that case, border-left would be styled with '1px dotted ThreeDShadow' even
in
> RTL, wouldn't it? 

You are correct. I missed that.

>  
> > html[dir='rtl'] th + th,
> > html[dir='rtl'] td + td {
> >     border-right: 1px dotted ThreeDShadow;
> > }

Powered by Google App Engine
This is Rietveld 408576698