Fix: issue with chrome v28 and below

The chrome 24v issue was mentioned in the link below:
https://adblockplus.org/forum/viewtopic.php?f=10&t=19973&start=15#p91792

After investigations looks like I found the reason and made some fix that worked for me with same chrome version reporter used (24.0.1312.52).

The case is about the sender parameter that onMessage callback parameter gets, so here what I've noticed:

common.js line: 109-112
ABP checks if chrome.runtime.sendMessage exists within installed chrome API and i case it’s not, ABP use chrome.extension.sendMessage (v24) for message passing.

common.js line: 71-74
Same way ABP check for the corresponding listener for message passing:
chrome.runtime.sendMessage  -> chrome.runtime.onMessage
chrome.extension.sendMessage-> chrome.extension.onMessage (v24)

But the sender parameter that the onMessage listener’s callback function is getting looks different:

* id
* url (missing in chrome.extension.sendMessage chrome v 24)
* tab

because of that missing parameter chrome v 24 and I guess also the older version who are not using chrome.runtime.onMessage are getting current error:
Error in event handler for 'undefined': Cannot call method 'replace' of undefined TypeError: Cannot call method 'replace' of undefined
    at stripFragmentFromURL (/lib/basedomain.js:165:14)

I’ve added current line to modify the sender object so it get tab’s url value:
ext/background.js line 189:
if(sender.url == null);
       sender.url = tab.url;

but actually in this case I’m getting the tab url not the sender’s one, so in case the request will be sent from Iframe I’ll again get the URL for tab itself.

[Thomas Greiner, 2014-02-10 11:05:34]

http://codereview.adblockplus.org/4538458420805632/diff/5629499534213120/chro...
File chrome/ext/background.js (right):

http://codereview.adblockplus.org/4538458420805632/diff/5629499534213120/chro...
chrome/ext/background.js:189: if (sender.url == null);
Add a comment above why this is necessary and investigate further which versions
are affected by this so that we know when it's safe to get rid of it again.

e.g. "Chrome 21-26 does not pass sender.url"

[saroyanm, 2014-02-12 12:22:15]

On 2014/02/10 11:05:34, Thomas Greiner wrote:
>
http://codereview.adblockplus.org/4538458420805632/diff/5629499534213120/chro...
> File chrome/ext/background.js (right):
> 
>
http://codereview.adblockplus.org/4538458420805632/diff/5629499534213120/chro...
> chrome/ext/background.js:189: if (sender.url == null);
> Add a comment above why this is necessary and investigate further which
versions
> are affected by this so that we know when it's safe to get rid of it again.
> 
> e.g. "Chrome 21-26 does not pass sender.url"

Got this group discussion regarding when the feature was added, will test it by
myself and update the patch:
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-extensions/HNN2...

[saroyanm, 2014-02-13 14:42:56]

> Add a comment above why this is necessary and investigate further which
versions
> are affected by this so that we know when it's safe to get rid of it again.
> 
Hope this will work.

[Felix Dahlke, 2014-02-27 20:45:24]

LGTM with a comment

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
File chrome/ext/background.js (right):

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
chrome/ext/background.js:191: //It's a temporary fix (
http://codereview.adblockplus.org/4538458420805632/ )
I'd get rid of this line, doesn't add much. The first one makes sense of course.

[Felix Dahlke, 2014-02-27 20:46:39]

Spotted something after hitting "publish" :P

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
File chrome/ext/background.js (right):

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
chrome/ext/background.js:192: if (sender.url == null);
Actually, why check for null here? Isn't the url parameter missing on Chrome
<28? In that case it should be:

if (!("url in sender))

[saroyanm, 2014-02-28 12:47:15]

Guys what you think should this be somehow commented in the blog post ?
In case should this comment be done by mapx, or I can comment ?
Or that should be commented after release ?

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
File chrome/ext/background.js (right):

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
chrome/ext/background.js:191: //It's a temporary fix (
http://codereview.adblockplus.org/4538458420805632/ )
On 2014/02/27 20:45:25, Felix H. Dahlke wrote:
> I'd get rid of this line, doesn't add much. The first one makes sense of
course.

Done.

http://codereview.adblockplus.org/4538458420805632/diff/5194384987389952/chro...
chrome/ext/background.js:192: if (sender.url == null);
On 2014/02/27 20:46:39, Felix H. Dahlke wrote:
> Actually, why check for null here? Isn't the url parameter missing on Chrome
> <28? In that case it should be:
> 
> if (!("url in sender))

Thanks for pointing.

[Felix Dahlke, 2014-02-28 12:50:50]

On 2014/02/28 12:47:15, saroyanm wrote:
> Guys what you think should this be somehow commented in the blog post ?
> In case should this comment be done by mapx, or I can comment ?
> Or that should be commented after release ?

You mean on the forum? You can say that it's fixed and point to the commit that
fixes it.

It'll be mentioned in the next release blog post, probably next week.