Posted by gbrussel on November 5, 2009 at 4:20pm
| Project: | Service links |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | JavaScript, safari, service_links, window.external |
Issue Summary
There is a typo on line 4 of favorite_services.js that is causing inconsistencies.
if (window.sidebar || window.external.AddFavourite) { should be if (window.sidebar || window.external.AddFavorite) {
Also, I have been testing in Chrome and it is not displaying. The comments in this file say it should only display for FF/IE, any reason why Opera/Chrome/Safari have been neglected?
Comments
#1
Correction. That line should read:
if (window.sidebar || window.external) {This now works in IE 7, FF3 and Chrome. Haven't tested Opera/Safari/IE8/IE6.
#2
Not working on Safari/Leopard
#3
This error comes up on Mac with firebug (service links was breaking things on the Ubercart checkout page on a Mac in Safari, not FF, and turning it off fixed it)
service_links/js/favorite_services.js --> on line 4 TypeError: Result of expression 'window.external' [undefined] is not an object.
#4
I found the problem. The code on line 4 is testing for window.external.AddFavourite, but on Safari window.external is not an object, so you can't test for window.external.AddFavourite. You first need to test for window.external all by its lonesome. I've changed my code to the following and it works fine.
if (window.sidebar || window.external) { // this line is new
if (window.sidebar || window.external.AddFavourite) {
$("a.service_links_favorite").show();
$("a.service_links_favorite").click(function(event){
event.preventDefault();
var url = unescape(this.href.replace(/\+/g,' '));
var url = url.replace(/^[^\?]*\?/g, "");
var title = url.replace(/^[^#]*#/g,"");
url = url.replace(/#.*$/g, "");
if (window.sidebar) {
window.sidebar.addPanel(title, url,"");
} else if ( window.external ) {
window.external.AddFavorite( url, title);
}
});
} else { // this line is new
$("a.service_links_favorite").hide(); // this line is new
} // this line is new
} else {
$("a.service_links_favorite").hide();
}
#5
Here's the above code in a patch. In case people don't know how to patch, just cd to the js directory and run
patch -p0 < name_of_patch_hereThis fixed my Javascript and made it work on my site in Safari 4
#6
Added the support for Opera and Chrome, and Safari shouldn't have problems with it.
About Chrome it show just a message who ask to press CTRL + D, if someone has a better idea i'll be happy to integrate it.
No problem to support more browser but not alls allow the automatic bookmarking through javascript (this is the case of Chrome).
#7
Automatically closed -- issue fixed for 2 weeks with no activity.
#8
I had a syntax error with the patch.
This resolves the problem:
Replace:
if (window.sidebar || window.external.AddFavourite) {By:
if (window.sidebar || window.external || window.external.AddFavourite) {