Chromium Code Reviews
Help | Chromium Project | Sign in
(451)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by Jungshik Shin
Modified:
2 years, 11 months ago
Reviewers:
arv
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) Lint Patch
M chrome/browser/browser_about_handler.cc View 3 chunks +11 lines, -16 lines 1 comment 0 errors Download
M chrome/browser/resources/about_plugins.html View 4 chunks +19 lines, -9 lines 1 comment 0 errors Download
Commit:

Messages

Total messages: 4
Jungshik Shin
4 years, 3 months ago #1
arv
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 ...
4 years, 3 months ago #2
Jungshik Shin
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 > ...
4 years, 3 months ago #3
arv
4 years, 3 months ago #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;
> > }
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6