Always enable copy-link-address in context menu.

Always enable copy-link-address in context menu.

Added a new field into ContextMenuParam only for this purpose. The new field won't be validated in frontend processes. In this way, even if renderer processes are going to mad, the frontend would be OK if the frontend uses this new field only for copying into the clipboard.

BUG=2725

[hamaji, 2009-04-21 20:35:58]

Hi,

Could you review this change? I'm not sure if this change is the best way to fix
this bug. If you have better idea to fix this, please let me know.

Thanks!

[Evan Stade, 2009-04-21 22:18:50]

If this works, I don't understand why it works, because it looks like link_url
and link_url_for_copy are exactly the same, so .is_valid() should be the same on
both of them. In any case it seems extraneous to add this field. See below
comment.

http://codereview.chromium.org/91002/diff/1/3
File chrome/browser/tab_contents/render_view_context_menu.cc (right):

http://codereview.chromium.org/91002/diff/1/3#newcode226
Line 226: return params_.link_url_for_copy.is_valid();
Just make this line

  return params_.link_url.is_empty()

and scrap link_url_for_copy. Or is there something I'm missing?

[hamaji, 2009-04-21 22:29:12]

It works (and is necessary) because |params.link_url| is modified in
RenderViewHost::OnMsgContextMenu() by FilterURL(). I added comments on 
chrome/browser/renderer_host/render_view_host.cc .

On 2009/04/21 22:18:50, Evan Stade wrote:
> If this works, I don't understand why it works, because it looks like link_url
> and link_url_for_copy are exactly the same, so .is_valid() should be the same
on
> both of them. In any case it seems extraneous to add this field. See below
> comment.
> 
> http://codereview.chromium.org/91002/diff/1/3
> File chrome/browser/tab_contents/render_view_context_menu.cc (right):
> 
> http://codereview.chromium.org/91002/diff/1/3#newcode226
> Line 226: return params_.link_url_for_copy.is_valid();
> Just make this line
> 
>   return params_.link_url.is_empty()
> 
> and scrap link_url_for_copy. Or is there something I'm missing?

[Evan Stade, 2009-04-21 22:39:28]

Aha, thanks for the explanation. I'm CCing brettw and jam because they'd
probably know better than I (a) whether this is the right fix and (b) whether
this should be fixed.

[hamaji, 2009-04-21 22:45:44]

Thanks for CCing this, and sorry for lacking information in my first
description. I do think this fix looks not so good, but I'm sure this feature is
useful if there are no security concern.

Thanks,

On 2009/04/21 22:39:28, Evan Stade wrote:
> Aha, thanks for the explanation. I'm CCing brettw and jam because they'd
> probably know better than I (a) whether this is the right fix and (b) whether
> this should be fixed.

[brettw, 2009-04-22 17:50:28]

I think overall this is the right approach.

http://codereview.chromium.org/91002/diff/3003/3004
File webkit/glue/context_menu.h (right):

http://codereview.chromium.org/91002/diff/3003/3004#newcode79
Line 79: GURL link_url_for_copy;
What do you think about something like "original_link_url" or
"unfiltered_link_url"? For copy sounds weird to me.

[hamaji, 2009-04-22 18:28:44]

Thank you for the review!

http://codereview.chromium.org/91002/diff/3003/3004
File webkit/glue/context_menu.h (right):

http://codereview.chromium.org/91002/diff/3003/3004#newcode79
Line 79: GURL link_url_for_copy;
On 2009/04/22 17:50:28, brettw wrote:
> What do you think about something like "original_link_url" or
> "unfiltered_link_url"? For copy sounds weird to me.

Thanks. I like unfiltered_link_url and fixed the code. At first, I used
original_link_url actually, and I renamed to link_url_for_copy. As I thought
using this value may introduce security issues into frontend process, I wanted
to warn people by the name of this field. I think "unfilter_link_url" can be
such warning, too.

[brettw, 2009-04-22 19:26:18]

LGTM. I also like unfiltered.

[hamaji, 2009-04-22 19:34:59]

On 2009/04/22 19:26:18, brettw wrote:
> LGTM. I also like unfiltered.

Thanks you very much! Could you check this in? I don't have permission of SVN
write access yet.