Commit 4f56da47 authored by lgrey's avatar lgrey Committed by Commit bot

There's a bunch of weird glitches right now that cause the text to flip to the...

There's a bunch of weird glitches right now that cause the text to flip to the left. AFAICT, these two changes fix all of them:
1) Set the alignment of the text field directly.
2) RFC 3987 requires us to set the writing direction of URLs to LTR so that we get שלום.com instead of moc.שלום. This appears to change the alignment of the string if it's not set explicitly. So: set it explicitly.

BUG=648554, 673362

Review-Url: https://codereview.chromium.org/2555783002
Cr-Commit-Position: refs/heads/master@{#438529}
parent 70c50537
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#import "base/mac/sdk_forward_declarations.h" #import "base/mac/sdk_forward_declarations.h"
#include "chrome/browser/themes/theme_service.h" #include "chrome/browser/themes/theme_service.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h" #import "chrome/browser/ui/cocoa/browser_window_controller.h"
#include "chrome/browser/ui/cocoa/l10n_util.h"
#import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.h" #import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.h"
#import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h" #import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h"
#import "chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h" #import "chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h"
...@@ -46,6 +47,9 @@ const CGFloat kAnimationDuration = 0.2; ...@@ -46,6 +47,9 @@ const CGFloat kAnimationDuration = 0.2;
resizeAnimation_.reset([[NSViewAnimation alloc] init]); resizeAnimation_.reset([[NSViewAnimation alloc] init]);
[resizeAnimation_ setDuration:kAnimationDuration]; [resizeAnimation_ setDuration:kAnimationDuration];
[resizeAnimation_ setAnimationBlockingMode:NSAnimationNonblocking]; [resizeAnimation_ setAnimationBlockingMode:NSAnimationNonblocking];
[self setAlignment:cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
? NSRightTextAlignment
: NSLeftTextAlignment];
// Disable Force Touch in the Omnibox. Note that this API is defined in // Disable Force Touch in the Omnibox. Note that this API is defined in
// 10.10.3 and higher so have to check more than just isYosmiteOrLater(). // 10.10.3 and higher so have to check more than just isYosmiteOrLater().
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <memory> #include <memory>
#include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.h" #include "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.h"
...@@ -133,6 +134,8 @@ class OmniboxViewMac : public OmniboxView, ...@@ -133,6 +134,8 @@ class OmniboxViewMac : public OmniboxView,
AutocompleteTextField* field() const { return field_; } AutocompleteTextField* field() const { return field_; }
private: private:
FRIEND_TEST_ALL_PREFIXES(OmniboxViewMacTest, WritingDirectionLTR);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewMacTest, WritingDirectionRTL);
// Called when the user hits backspace in |field_|. Checks whether // Called when the user hits backspace in |field_|. Checks whether
// keyword search is being terminated. Returns true if the // keyword search is being terminated. Returns true if the
// backspace should be intercepted (not forwarded on to the standard // backspace should be intercepted (not forwarded on to the standard
......
...@@ -17,7 +17,8 @@ ...@@ -17,7 +17,8 @@
#include "chrome/browser/search/search.h" #include "chrome/browser/search/search.h"
#include "chrome/browser/themes/theme_service.h" #include "chrome/browser/themes/theme_service.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h" #import "chrome/browser/ui/cocoa/browser_window_controller.h"
#include "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.h" #import "chrome/browser/ui/cocoa/l10n_util.h"
#import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.h"
#import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h" #import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h"
#include "chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h" #include "chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h"
#include "chrome/browser/ui/omnibox/chrome_omnibox_client.h" #include "chrome/browser/ui/omnibox/chrome_omnibox_client.h"
...@@ -530,6 +531,10 @@ void OmniboxViewMac::ApplyTextStyle( ...@@ -530,6 +531,10 @@ void OmniboxViewMac::ApplyTextStyle(
[paragraph_style setMaximumLineHeight:line_height]; [paragraph_style setMaximumLineHeight:line_height];
[paragraph_style setMinimumLineHeight:line_height]; [paragraph_style setMinimumLineHeight:line_height];
[paragraph_style setLineBreakMode:NSLineBreakByTruncatingTail]; [paragraph_style setLineBreakMode:NSLineBreakByTruncatingTail];
// Set an explicit alignment so it isn't implied from writing direction.
[paragraph_style setAlignment:cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
? NSRightTextAlignment
: NSLeftTextAlignment];
// If this is a URL, set the top-level paragraph direction to LTR (avoids RTL // If this is a URL, set the top-level paragraph direction to LTR (avoids RTL
// characters from making the URL render from right to left, as per RFC 3987 // characters from making the URL render from right to left, as per RFC 3987
// Section 4.1). // Section 4.1).
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "chrome/browser/ui/cocoa/test/cocoa_profile_test.h" #include "chrome/browser/ui/cocoa/test/cocoa_profile_test.h"
#include "chrome/browser/ui/cocoa/test/scoped_force_rtl_mac.h"
#include "chrome/browser/ui/omnibox/chrome_omnibox_client.h" #include "chrome/browser/ui/omnibox/chrome_omnibox_client.h"
#include "chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h" #include "chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h"
#include "chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.h" #include "chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.h"
...@@ -176,3 +177,65 @@ TEST_F(OmniboxViewMacTest, UpDownArrow) { ...@@ -176,3 +177,65 @@ TEST_F(OmniboxViewMacTest, UpDownArrow) {
view.OnDoCommandBySelector(@selector(moveUp:)); view.OnDoCommandBySelector(@selector(moveUp:));
EXPECT_EQ(-1, model->up_or_down_count()); EXPECT_EQ(-1, model->up_or_down_count());
} }
TEST_F(OmniboxViewMacTest, WritingDirectionLTR) {
TestingToolbarModelDelegate delegate;
ToolbarModelImpl toolbar_model(&delegate, 32 * 1024);
TestingOmniboxEditController edit_controller(&toolbar_model);
OmniboxViewMac view(&edit_controller, profile(), NULL, NULL);
// This is deleted by the omnibox view.
MockOmniboxEditModel* model =
new MockOmniboxEditModel(&view, &edit_controller, profile());
MockOmniboxPopupView popup_view;
OmniboxPopupModel popup_model(&popup_view, model);
model->OnSetFocus(true);
SetModel(&view, model);
view.SetUserText(base::ASCIIToUTF16("foo.com"));
model->OnChanged();
base::scoped_nsobject<NSMutableAttributedString> string(
[[NSMutableAttributedString alloc] initWithString:@"foo.com"]);
view.ApplyTextStyle(string);
NSParagraphStyle* paragraphStyle =
[string attribute:NSParagraphStyleAttributeName
atIndex:0
effectiveRange:NULL];
DCHECK(paragraphStyle);
EXPECT_EQ(paragraphStyle.alignment, NSLeftTextAlignment);
EXPECT_EQ(paragraphStyle.baseWritingDirection, NSWritingDirectionLeftToRight);
}
TEST_F(OmniboxViewMacTest, WritingDirectionRTL) {
cocoa_l10n_util::ScopedForceRTLMac rtl;
TestingToolbarModelDelegate delegate;
ToolbarModelImpl toolbar_model(&delegate, 32 * 1024);
TestingOmniboxEditController edit_controller(&toolbar_model);
OmniboxViewMac view(&edit_controller, profile(), NULL, NULL);
// This is deleted by the omnibox view.
MockOmniboxEditModel* model =
new MockOmniboxEditModel(&view, &edit_controller, profile());
MockOmniboxPopupView popup_view;
OmniboxPopupModel popup_model(&popup_view, model);
model->OnSetFocus(true);
SetModel(&view, model);
view.SetUserText(base::ASCIIToUTF16("foo.com"));
model->OnChanged();
base::scoped_nsobject<NSMutableAttributedString> string(
[[NSMutableAttributedString alloc] initWithString:@"foo.com"]);
view.ApplyTextStyle(string);
NSParagraphStyle* paragraphStyle =
[string attribute:NSParagraphStyleAttributeName
atIndex:0
effectiveRange:NULL];
DCHECK(paragraphStyle);
EXPECT_EQ(paragraphStyle.alignment, NSRightTextAlignment);
EXPECT_EQ(paragraphStyle.baseWritingDirection, NSWritingDirectionLeftToRight);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment