diff options
| author | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-04-27 03:33:59 +0000 | 
|---|---|---|
| committer | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-04-27 03:33:59 +0000 | 
| commit | 562b222ad53049a87f695e450e8698d3cb2aa1d6 (patch) | |
| tree | 7ed368560ed1137f330bf1b3d98df84e95107cc8 | |
| parent | 7ff213014f83fc4066fb80f7c4ca3993d81a6b37 (diff) | |
| download | chromium_src-562b222ad53049a87f695e450e8698d3cb2aa1d6.zip chromium_src-562b222ad53049a87f695e450e8698d3cb2aa1d6.tar.gz chromium_src-562b222ad53049a87f695e450e8698d3cb2aa1d6.tar.bz2 | |
Tweaks to copy/paste of omnibox on windows:
. Makes copy prefix the text with http if the user hasn't edited the
  text, and the scheme is http but the text doesn't include http.
. Makes copying use the url as the text if the user hasn't edited the
  text and it's http/https.
. Makes control-insert/shift-insert use our copy/paste.
BUG=41639 41489 41493
TEST=make sure copy/paste from the omnibox work sanely. Seriously
     though, see the three bugs for specifics.
Review URL: http://codereview.chromium.org/1761002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@45667 0039d316-1c4b-4281-b951-d872f2087c98
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit.cc | 41 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit.h | 11 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit_unittest.cc | 124 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit_view_win.cc | 48 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 | 
5 files changed, 194 insertions, 31 deletions
| diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index 9b0aefb..5d68e59 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -7,6 +7,7 @@  #include <string>  #include "base/basictypes.h" +#include "base/string_util.h"  #include "base/utf_string_conversions.h"  #include "chrome/app/chrome_dll_resource.h"  #include "chrome/browser/autocomplete/autocomplete_classifier.h" @@ -21,6 +22,7 @@  #include "chrome/browser/search_engines/template_url.h"  #include "chrome/browser/search_engines/template_url_model.h"  #include "chrome/common/notification_service.h" +#include "chrome/common/url_constants.h"  #include "googleurl/src/gurl.h"  #include "googleurl/src/url_util.h"  #include "third_party/skia/include/core/SkBitmap.h" @@ -165,6 +167,45 @@ bool AutocompleteEditModel::GetURLForText(const std::wstring& text,    return true;  } +void AutocompleteEditModel::AdjustTextForCopy(int sel_start, +                                              bool is_all_selected, +                                              std::wstring* text, +                                              GURL* url, +                                              bool* write_url) { +  *write_url = false; + +  if (sel_start != 0) +    return; + +  // We can't use CurrentTextIsURL() or GetDataForURLExport() because right now +  // the user is probably holding down control to cause the copy, which will +  // screw up our calculation of the desired_tld. +  if (!GetURLForText(*text, url)) +    return;  // Can't be parsed as a url, no need to adjust text. + +  if (!user_input_in_progress() && is_all_selected) { +    *text = UTF8ToWide(url->spec()); +    *write_url = true; +    return; +  } + +  // Prefix the text with 'http://' if the text doesn't start with 'http://', +  // the text parses as a url with a scheme of http, the user selected the +  // entire host, and the user hasn't edited the host or manually removed the +  // scheme. +  if (url->SchemeIs(chrome::kHttpScheme)) { +    std::wstring http = ASCIIToWide(chrome::kHttpScheme) + +        ASCIIToWide(chrome::kStandardSchemeSeparator); +    std::wstring host = UTF8ToWide(url->host()); +    if (text->compare(0, http.length(), http) != 0 && +        text->length() >= host.length() && +        permanent_text_.compare(0, host.length(), host) == 0) { +      *text = http + *text; +      *write_url = true; +    } +  } +} +  void AutocompleteEditModel::SetInputInProgress(bool in_progress) {    if (user_input_in_progress_ == in_progress)      return; diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 9f4e973..66d6b98 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -147,6 +147,17 @@ class AutocompleteEditModel : public NotificationObserver {    // copy.    bool GetURLForText(const std::wstring& text, GURL* url) const; +  // Invoked to adjust the text before writting to the clipboard for a copy +  // (e.g. by adding 'http' to the front). |sel_start| gives the start of the +  // selection. |text| is the currently selected text. If |is_all_selected| is +  // true all the text in the edit is selected. If the url should be copied to +  // the clipboard |write_url| is set to true and |url| set to the url to write. +  void AdjustTextForCopy(int sel_start, +                         bool is_all_selected, +                         std::wstring* text, +                         GURL* url, +                         bool* write_url); +    bool user_input_in_progress() const { return user_input_in_progress_; }    // Sets the state of user_input_in_progress_, and notifies the observer if diff --git a/chrome/browser/autocomplete/autocomplete_edit_unittest.cc b/chrome/browser/autocomplete/autocomplete_edit_unittest.cc new file mode 100644 index 0000000..8affff1 --- /dev/null +++ b/chrome/browser/autocomplete/autocomplete_edit_unittest.cc @@ -0,0 +1,124 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/autocomplete/autocomplete_edit.h" +#include "chrome/browser/autocomplete/autocomplete_edit_view.h" +#include "chrome/test/testing_profile.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class TestingAutocompleteEditView : public AutocompleteEditView { + public: +  TestingAutocompleteEditView() {} + +  virtual AutocompleteEditModel* model() { return NULL; } +  virtual const AutocompleteEditModel* model() const { return NULL; } +  virtual void SaveStateToTab(TabContents* tab) {} +  virtual void Update(const TabContents* tab_for_state_restoring) {} +  virtual void OpenURL(const GURL& url, +                       WindowOpenDisposition disposition, +                       PageTransition::Type transition, +                       const GURL& alternate_nav_url, +                       size_t selected_line, +                       const std::wstring& keyword) {} +  virtual std::wstring GetText() const { return std::wstring(); } +  virtual bool IsEditingOrEmpty() const { return true; } +  virtual int GetIcon() const { return 0; } +  virtual void SetUserText(const std::wstring& text) {} +  virtual void SetUserText(const std::wstring& text, +                           const std::wstring& display_text, +                           bool update_popup) {} +  virtual void SetWindowTextAndCaretPos(const std::wstring& text, +                                        size_t caret_pos) {} +  virtual void SetForcedQuery() {} +  virtual bool IsSelectAll() { return false; } +  virtual void SelectAll(bool reversed) {} +  virtual void RevertAll() {} +  virtual void UpdatePopup() {} +  virtual void ClosePopup() {} +  virtual void SetFocus() {} +  virtual void OnTemporaryTextMaybeChanged(const std::wstring& display_text, +                                           bool save_original_selection) {} +  virtual bool OnInlineAutocompleteTextMaybeChanged( +      const std::wstring& display_text, size_t user_text_length) { +    return false; +  } +  virtual void OnRevertTemporaryText() {} +  virtual void OnBeforePossibleChange() {} +  virtual bool OnAfterPossibleChange() { return false; } +  virtual gfx::NativeView GetNativeView() const { return 0; } +  virtual CommandUpdater* GetCommandUpdater() { return NULL; } + + private: +  DISALLOW_COPY_AND_ASSIGN(TestingAutocompleteEditView); +}; + +class TestingAutocompleteEditController : public AutocompleteEditController { + public: +  TestingAutocompleteEditController() {} +  virtual void OnAutocompleteAccept(const GURL& url, +                                    WindowOpenDisposition disposition, +                                    PageTransition::Type transition, +                                    const GURL& alternate_nav_url) {} +  virtual void OnChanged() {} +  virtual void OnInputInProgress(bool in_progress) {} +  virtual void OnKillFocus() {} +  virtual void OnSetFocus() {} +  virtual SkBitmap GetFavIcon() const { return SkBitmap(); } +  virtual std::wstring GetTitle() const { return std::wstring(); } + + private: +  DISALLOW_COPY_AND_ASSIGN(TestingAutocompleteEditController); +}; + +} + +typedef testing::Test AutocompleteEditTest; + +// Tests various permutations of AutocompleteModel::AdjustTextForCopy. +TEST(AutocompleteEditTest, AdjustTextForCopy) { +  struct Data { +    const int sel_start; +    const bool is_all_selected; +    const wchar_t* input; +    const wchar_t* expected_output; +    const bool write_url; +    const char* expected_url; +  } input[] = { +    // Test that http:// is inserted if all text is selected. +    { 0, true,  L"a.b/c", L"http://a.b/c", true,  "http://a.b/c" }, + +    // Test that http:// is inserted if the host is selected. +    { 0, false, L"a.b/",  L"http://a.b/",  true,  "http://a.b/" }, + +    // Tests that http:// is inserted if the path is modified. +    { 0, false, L"a.b/d", L"http://a.b/d", true,  "http://a.b/d" }, + +    // Tests that http:// isn't inserted if the host is modified. +    { 0, false, L"a.c/",  L"a.c/",         false, "" }, + +    // Tests that http:// isn't inserted if the start of the selection is 1. +    { 1, false, L"a.b/",  L"a.b/",         false, "" }, +  }; +  TestingAutocompleteEditView view; +  TestingAutocompleteEditController controller; +  TestingProfile profile; +  AutocompleteEditModel model(&view, &controller, &profile); + +  model.UpdatePermanentText(L"a.b/c"); + +  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(input); ++i) { +    std::wstring result(input[i].input); +    GURL url; +    bool write_url; +    model.AdjustTextForCopy(input[i].sel_start, input[i].is_all_selected, +                            &result, &url, &write_url); +    EXPECT_EQ(input[i].expected_output, result) << "@: " << i; +    EXPECT_EQ(input[i].write_url, write_url) << " @" << i; +    if (write_url) +      EXPECT_EQ(input[i].expected_url, url.spec()) << " @" << i; +  } +} diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc index 5a0632f..153354c 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc @@ -1167,36 +1167,21 @@ void AutocompleteEditViewWin::OnContextMenu(HWND window, const CPoint& point) {  }  void AutocompleteEditViewWin::OnCopy() { -  const std::wstring text(GetSelectedText()); +  std::wstring text(GetSelectedText());    if (text.empty())      return; +  CHARRANGE sel; +  GURL url; +  bool write_url = false; +  GetSel(sel); +  model_->AdjustTextForCopy(sel.cpMin, IsSelectAll(), &text, &url, &write_url);    ScopedClipboardWriter scw(g_browser_process->clipboard()); -  // Check if the user is copying the whole address bar.  If they are, we -  // assume they are trying to copy a URL and write this to the clipboard as a -  // hyperlink. -  if (static_cast<int>(text.length()) >= GetTextLength()) { -    // The entire control is selected.  Let's see what the user typed.  We -    // can't use model_->CurrentTextIsURL() or model_->GetDataForURLExport() -    // because right now the user is probably holding down control to cause the -    // copy, which will screw up our calculation of the desired_tld. -    GURL url; -    if (model_->GetURLForText(text, &url)) { -      // If the scheme is http or https and the user isn't editing, -      // we should copy the true URL instead of the (unescaped) display -      // string to avoid encoding and escaping issues when pasting this text -      // elsewhere. -      if ((url.SchemeIs("http") || url.SchemeIs("https")) && -          !model_->user_input_in_progress()) -        scw.WriteText(UTF8ToWide(url.spec())); -      else -        scw.WriteText(text); -      scw.WriteBookmark(text, url.spec()); -      scw.WriteHyperlink(EscapeForHTML(UTF16ToUTF8(text)), url.spec()); -      return; -    } -  }    scw.WriteText(text); +  if (write_url) { +    scw.WriteBookmark(text, url.spec()); +    scw.WriteHyperlink(EscapeForHTML(UTF16ToUTF8(text)), url.spec()); +  }  }  void AutocompleteEditViewWin::OnCut() { @@ -1794,10 +1779,10 @@ bool AutocompleteEditViewWin::OnKeyDownOnlyWritable(TCHAR key,      // Cut:   Shift-Delete and Ctrl-x are treated as cut.  Ctrl-Shift-Delete and      //        Ctrl-Shift-x are not treated as cut even though the underlying      //        CRichTextEdit would treat them as such. -    // Copy:  Ctrl-c is treated as copy.  Shift-Ctrl-c is not.  (This is handled -    //        in OnKeyDownAllModes().) -    // Paste: Shift-Insert and Ctrl-v are tread as paste.  Ctrl-Shift-Insert and -    //        Ctrl-Shift-v are not. +    // Copy:  Ctrl-Insert and Ctrl-c is treated as copy.  Shift-Ctrl-c is not. +    //        (This is handled in OnKeyDownAllModes().) +    // Paste: Shift-Insert and Ctrl-v are treated as paste.  Ctrl-Shift-Insert +    //        and Ctrl-Shift-v are not.      //      // This behavior matches most, but not all Windows programs, and largely      // conforms to what users expect. @@ -1897,15 +1882,16 @@ bool AutocompleteEditViewWin::OnKeyDownOnlyWritable(TCHAR key,  bool AutocompleteEditViewWin::OnKeyDownAllModes(TCHAR key,                                                  UINT repeat_count,                                                  UINT flags) { -  // See KF_ALTDOWN comment atop OnKeyDownOnlyWriteable(). +  // See KF_ALTDOWN comment atop OnKeyDownOnlyWritable().    switch (key) {      case VK_CONTROL:        model_->OnControlKeyChanged(true);        return false; +    case VK_INSERT:      case 'C': -      // See more detailed comments in OnKeyDownOnlyWriteable(). +      // See more detailed comments in OnKeyDownOnlyWritable().        if ((flags & KF_ALTDOWN) || (GetKeyState(VK_CONTROL) >= 0))          return false;        if (GetKeyState(VK_SHIFT) >= 0) diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index f8d15b3..ccfe83d 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -545,6 +545,7 @@          # All unittests in browser, common, and renderer.          'browser/app_controller_mac_unittest.mm',          'browser/app_menu_model_unittest.cc', +        'browser/autocomplete/autocomplete_edit_unittest.cc',          'browser/autocomplete/autocomplete_edit_view_mac_unittest.mm',          'browser/autocomplete/autocomplete_unittest.cc',          'browser/autocomplete/autocomplete_popup_view_mac_unittest.mm', | 
