New issue
Advanced search Search tips

Issue 597382 link

Starred by 2 users
Status: Verified
Owner:
thomasanderson@chromium.org
Closed: Feb 2017
Cc:
msw@chromium.org
thomasanderson@chromium.org
dpranke@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Readonly Omnibox allows paste

Project Member Reported by elawrence@chromium.org, Mar 23 2016 
Version: 51.0.2688.0
OS: Win7

What steps will reproduce the problem?
(1) Visit http://ericlawrence.com. 
(2) Click a comment link to do a window.open with "location=0" in the options string
(3) In the window that opens, paste a URL in the omnibox and hit enter.

What is the expected output?

Expect either paste is blocked (since omnibox is readonly) or paste is allowed and hitting enter navigates.

What do you see instead?

URL pastes into box. Instead of navigating, hitting Enter deletes the pasted URL and the text reverts to the current URL.

(Repro Win,CrOs. Not repro on OSX)

Comment 1 by pkasting@chromium.org, Mar 23 2016

Cc: msw@chromium.org pkasting@chromium.org
Labels: OS-Linux
Status: Available (was: Untriaged)
Summary: Readonly Omnibox allows paste (was: Readonly Omnibox allows paste, but navigation does not occur)
Might be a bug in the underlying Textfield.

Comment 2 by dpranke@chromium.org, Mar 23 2016

Cc: dpranke@chromium.org
I have someone who might work on this ... will cc him as soon as we get him an account.

Comment 3 by elawrence@chromium.org, Mar 24 2016 

IsCommandIdEnabled(IDS_APP_PASTE) checks if |editable|
https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/controls/textfield/textfield.cc&sq=package:chromium&type=cs&l=1245&rcl=1458819216

but OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) does not check it:

  case IDS_APP_PASTE:
      OnPaste();
      return;
    default:
      if (Textfield::IsCommandIdEnabled(command_id))

The following patch seems to resolve the problem:

--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -370,7 +370,9 @@ void OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) {

     // These commands do invoke the popup.
     case IDS_APP_PASTE:
+      if (Textfield::IsCommandIdEnabled(command_id)) {
         OnPaste();
+      }
       return;

Comment 4 by pkasting@chromium.org, Mar 24 2016 

Sounds like this will be simple to review and land.  You'll want to follow the instructions on http://dev.chromium.org/developers/contributing-code for how to proceed with getting your patch landed.

Comment 5 by elawrence@chromium.org, Mar 25 2016

Owner: elawrence@chromium.org
Status: Started (was: Available)
https://codereview.chromium.org/1833133002/

Comment 6 by dpranke@chromium.org, Apr 29 2016

Cc: thomasanderson@chromium.org

Comment 7 Deleted

Comment 8 by thomasanderson@chromium.org, Apr 29 2016

Owner: thomasanderson@chromium.org
►

Sign in to add a comment