omnibox: Enable or disable the copy menu item correctly.

omnibox: Enable or disable the copy menu item correctly.

BUG=29092
TEST=see bug

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35492

[tfarina (gmail-do not use), 2009-12-31 19:09:12]

Hi Finnur, could you review this to me?

[Finnur, 2009-12-31 20:58:19]

I'm not a fan of this; this seems like the wrong fix to me (adding pkasting who
is the Omnibox expert). 

The Omnibox for the popup is read-only, but should allow Copy when something is
selected and Copy should be grayed out otherwise.

In my opinion the right fix would be to implement (already implemented?) and
enable Copy for this menu and show Cut and Paste as grayed out, which indicates
to the user that the Omnibox is read-only.

[Peter Kasting, 2010-01-01 01:26:19]

Finnur is correct that even popup window omnibox context menus should allow copy
when something is selected, so that's what the bug is.

I would prefer not to add Cut and Paste to the popup window context menu because
they can never be enabled.  Adding them seems to imply that there is some
circumstance where they might be available.

[tfarina (gmail-do not use), 2010-01-01 14:59:38]

omnibox: Enable or disable the copy menu item correctly.

[tfarina (gmail-do not use), 2010-01-01 15:02:56]

Sorry for the last email, I was updating the description, but I pasted it on the
wrong location :(

I guess it was a typo, because it was passing IDS_COPY in the first paramenter
of AddItemWithStringId instead of IDC, and the verification on
IsCommandIdEnabled is expecting IDS_COPY.

[tfarina (gmail-do not use), 2010-01-01 15:03:42]

On 2010/01/01 15:02:56, tfarina wrote:
> Sorry for the last email, I was updating the description, but I pasted it on
the
> wrong location :(
> 
> I guess it was a typo, because it was passing IDS_COPY in the first paramenter
> of AddItemWithStringId instead of IDC, and the verification on
> IsCommandIdEnabled is expecting IDS_COPY.
s/IDS_COPY/IDC_COPY.

[Finnur, 2010-01-01 15:59:17]

LGTM


On Fri, Jan 1, 2010 at 07:03, <thiago.farina@gmail.com> wrote:

> On 2010/01/01 15:02:56, tfarina wrote:
>
>> Sorry for the last email, I was updating the description, but I pasted it
>> on
>>
> the
>
>> wrong location :(
>>
>
>  I guess it was a typo, because it was passing IDS_COPY in the first
>> paramenter
>> of AddItemWithStringId instead of IDC, and the verification on
>> IsCommandIdEnabled is expecting IDS_COPY.
>>
> s/IDS_COPY/IDC_COPY.
>
>
>
> http://codereview.chromium.org/522031
>

[Peter Kasting, 2010-01-01 20:49:11]

LGTM2

[tfarina (gmail-do not use), 2010-01-04 15:10:34]

On 2010/01/01 20:49:11, Peter Kasting wrote:

Peter could you land this to me?

[Peter Kasting, 2010-01-04 23:27:06]

Landed in r35492.