ContextMenu shows about:blank

As per the ContextMenuParam structure (context_menu_params.h) 
mLinkText may be an empty string if the contents of the link are an image.

So here as we are doing long press on image link the mLinkText is coming as empty string from nativeside.
And in buildContextMenu it sets header title as mLinkText so it sets "about:blank"

This patch checks if link are an image then sets title as source URL. 

BUG=385000
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279673

[Jitu( very slow this week), 2014-06-16 10:10:10]

PTAL

[kbalazs, 2014-06-18 20:40:57]

On 2014/06/16 10:10:10, Jitu wrote:
> PTAL

Change looks good, but imho you should document what you did not what was broken
in the description.

[Jitu( very slow this week), 2014-06-19 06:35:33]

On 2014/06/18 20:40:57, kbalazs wrote:
> On 2014/06/16 10:10:10, Jitu wrote:
> > PTAL
> 
> Change looks good, but imho you should document what you did not what was
broken
> in the description.

Updated the description

[bengr, 2014-06-19 12:46:24]

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
(right):

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:41:
if (!TextUtils.isEmpty(params.getLinkUrl()) &&
params.getLinkUrl().startsWith("http"))
Why is this limited to http?

[Jitu( very slow this week), 2014-06-19 14:31:38]

Please check my comment

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
(right):

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:41:
if (!TextUtils.isEmpty(params.getLinkUrl()) &&
params.getLinkUrl().startsWith("http"))
On 2014/06/19 12:46:24, bengr1 wrote:
> Why is this limited to http?

Thanks for this point.

Normally any LinkUrl starts with http or https, so here we are checking if it is
starts with http or https then 
show the LinkUrl.

Else if it is other than http or https then show the source URL of the image.

Do you think any other schema we need to check ?
Any real time use case ?

[bengr, 2014-06-19 15:15:53]

On 2014/06/19 14:31:38, Jitu wrote:
> Please check my comment
> 
>
https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
> (right):
> 
>
https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:41:
> if (!TextUtils.isEmpty(params.getLinkUrl()) &&
> params.getLinkUrl().startsWith("http"))
> On 2014/06/19 12:46:24, bengr1 wrote:
> > Why is this limited to http?
> 
> Thanks for this point.
> 
> Normally any LinkUrl starts with http or https, so here we are checking if it
is
> starts with http or https then 
> show the LinkUrl.
> 
> Else if it is other than http or https then show the source URL of the image.
> 
> Do you think any other schema we need to check ?
> Any real time use case ?

I see. Yaron, Marcus, do you think this change makes sense? lgtm, but I defer to
your opinions.

[Yaron, 2014-06-23 18:56:02]

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
(right):

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:41:
if (!TextUtils.isEmpty(params.getLinkUrl()) &&
params.getLinkUrl().startsWith("http"))
On 2014/06/19 14:31:38, Jitu wrote:
> On 2014/06/19 12:46:24, bengr1 wrote:
> > Why is this limited to http?
> 
> Thanks for this point.
> 
> Normally any LinkUrl starts with http or https, so here we are checking if it
is
> starts with http or https then 
> show the LinkUrl.
> 
> Else if it is other than http or https then show the source URL of the image.
> 
> Do you think any other schema we need to check ?
> Any real time use case ?
> 

What if the link started with a fragment? 
e.g.
<a href="#sometarget">link</a>
...
..
<a name="sometarget">target</a>

https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:43:
else if (!TextUtils.isEmpty(params.getSrcUrl()) && params.isImage())
Nit: make the params.isImage check first

[Jitu( very slow this week), 2014-06-24 12:35:33]

On 2014/06/23 18:56:02, Yaron wrote:
>
https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
> (right):
> 
>
https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:41:
> if (!TextUtils.isEmpty(params.getLinkUrl()) &&
> params.getLinkUrl().startsWith("http"))
> On 2014/06/19 14:31:38, Jitu wrote:
> > On 2014/06/19 12:46:24, bengr1 wrote:
> > > Why is this limited to http?
> > 
> > Thanks for this point.
> > 
> > Normally any LinkUrl starts with http or https, so here we are checking if
it
> is
> > starts with http or https then 
> > show the LinkUrl.
> > 
> > Else if it is other than http or https then show the source URL of the
image.
> > 
> > Do you think any other schema we need to check ?
> > Any real time use case ?
> > 
> 
> What if the link started with a fragment? 
> e.g.
> <a href="#sometarget">link</a>
> ...
> ..
> <a name="sometarget">target</a>
> 
>
https://codereview.chromium.org/339573002/diff/1/chrome/android/java/src/org/...
>
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:43:
> else if (!TextUtils.isEmpty(params.getSrcUrl()) && params.isImage())
> Nit: make the params.isImage check first

@Yaron,

In this patch i have taken care of all the schema and taken care all the
comments.
plz check.

Thanks.

[Yaron, 2014-06-24 17:23:37]

lgtm

[Jitu( very slow this week), 2014-06-25 06:13:33]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2014-06-25 06:16:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jitendra.ks@samsung.com/339573002/20001

[commit-bot: I haz the power, 2014-06-25 11:45:50]

Change committed as 279673