Comment #8 on issue 2821 by bugdro...@chromium.org: RTL: Ellipsis appeared
at right for RTL locales on "Task Manager - Google Chrome" dialog
http://code.google.com/p/chromium/issues/detail?id=2821
The following revision refers to this bug:
http://src.chromium.org/viewvc/chrome?view=rev&revision=8593
------------------------------------------------------------------------
r8593 | x...@chromium.org | 2009-01-23 15:13:05 -0800 (Fri, 23 Jan 2009) |
60 lines
Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/bookmarks/bookmark_table_model.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/task_manager.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/task_manager.h?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/task_manager_resource_providers.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/hung_renderer_view.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/keyword_editor_view.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/options/cookies_view.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/options/general_page_view.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/password_manager_view.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/shelf_item_dialog.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/l10n_util.h?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/views/native_control.cc?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/views/native_control.h?r1=8593&r2=8592
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/views/table_view.cc?r1=8593&r2=8592
===================================================
This change list fixes the following 2 chrome bugs:
2821 RTL: Ellipsis appeared at right for RTL locales on "Task Manager -
Google Chrome" dialog
(http://crbug.com/2821)
6132: Strings in tables are displayed LTR when they should be RTL
(http://crbug.com/6132)
===================================================
Background:
The above 2 bugs are related to Chrome task manager.
Task manager is a TableView, which is a subclass of NativeControl.
Currently, TableView window is created by using WS_EX_LAYOUTRTL |
WS_EX_RTLREADING flag, which is wrong.
WS_EX_LAYOUTRTL | WS_EX_RTLREADING renders a RTL alignment but LTR reading
order.
Please see the following blog for detail:
http://blogs.msdn.com/michkap/archive/2007/03/11/1857043.aspx
We need to replace the above flag with WS_EX_LAYOUTRTL only.
Consequently, we need to make sure that the various implementations of
TableModel::GetText adjust the
returned text appropriately if need be. For example, if the text in an URL,
then the returned text should be explicitly marked as LTR text.
Please note: there are several other places where we create RTL window
using WS_EX_LAYOUTRTL | WS_EX_RTLREADING flag, such as AutoCompleteEditView
(omnibox), MenuHostWindow,
TextField (for example, text in bookmark dialog), and other NativeControl
(including checkbox, combobox, native_button, radio_button, separator,
tabbed_pane, and tree view).
They all need to be fixed. And the following bug is filed for tracking.
http://crbug.com/6573
===================================================
The fix is based on Idan's comments on bug 6132
1.create the table view with WS_EX_LAYOUTRTL (rather than WS_EX_LAYOUTRTL |
WS_EX_RTLREADING)
2 Make sure that the various implementations of TableModel::GetText adjust
the returned text appropriately if need be.
If the text in an URL, then the returned text is explicitly marked as LTR
text.
Otherwise, the returned text is marked with LTR if it is RTL environment
and there is no strong RTL character in the text. This is to avoid the
wrong placement of (or wrong mirrored) ending-punctuation.
===================================================
Since we have quite a few places creating window using WS_EX_LAYOUTRTL |
WS_EX_RTLREADING style. We decided to migrate to the right style step to
step.
This is the first step, and there is unnecessary function introduced by
this change list.
1.
I am using l10n_util::GetExtendedTooltipStyles() for the right style for
now. And I marked a TODO there to obsolete this function name, replace with
GetExtendedStyles eventually. (the l10n_util.h is no longer in the change
list since I've checked it in with another CL).
2.
I introduced native_control::GetAdditionalRTLStyle() which should replace
GetAdditionalExStyle() eventually.
===================================================
When wrapping text with LTR format in GetText(),
ideally, we should parse the text to check whether this is a n URL or not
(for example, a webpage title could be an URL, or user could add URL as
search engine's name or keyword).
I did not do that and only added a TODO there because I think that might be
rare case and parsing the text might cause performance degradation.
Review URL: http://codereview.chromium.org/18076
------------------------------------------------------------------------
--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings
--~--~---------~--~----~------------~-------~--~----~
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/group/chromium-bugs
-~----------~----~----~----~------~----~------~--~---
