Bug 13102 - Javascript error messages on patagonia.com
Summary: Javascript error messages on patagonia.com
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Evangelism (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://www.patagonia.com
Keywords: HasReduction
Depends on:
Blocks:
 
Reported: 2007-03-17 09:11 PDT by Patricia Warwick
Modified: 2008-02-14 04:31 PST (History)
5 users (show)

See Also:


Attachments
Test case (partially reduced) (874 bytes, text/html)
2007-04-05 11:03 PDT, David Kilzer (:ddkilzer)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patricia Warwick 2007-03-17 09:11:43 PDT
When I use the patagonia site, I get a bunch of dialog windows with javascript errors. I do not get any errors with Safari and Firefox.

Here is the first message:

Something is erroring: 

File Name: TopNav.js
Function Name: InitializeTopNav
Message: TypeError: Value undefined (result of expression M_arrTopMenu.each) is not object., undefined

Refreshing your browser window may fix things.
Comment 1 Troy Brandt 2007-04-05 10:29:13 PDT
I came across this as well, it looks like it works in Safari Version 2.0.4 (419.3) so maybe we can get this marked as a regression. 
Comment 2 David Kilzer (:ddkilzer) 2007-04-05 10:58:24 PDT
The initial page (http://www.patagonia.com) has a parse error in the JavaScript console from the Prototype library using a local debug build of WebKit r20722 with Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135):

SyntaxError: Parse error
http://www.patagonia.com/usa/js/prototype.js     Line: 1807

The top of the prototype.js file says:

/*  Prototype JavaScript framework, version 1.4.0
 *  (c) 2005 Sam Stephenson <sam@conio.net>

My guess is that the site needs to update their version of the library (which is 1.5.0 as of this writing).

The code that's borken (which may or may not be a legitimate bug):

var Cookie = {
  set: function(name, value, daysToExpire) {
    var expire = '';
    if (daysToExpire != undefined) {
      var d = new Date();
      d.setTime(d.getTime() + (86400000 * parseFloat(daysToExpire)));
      expire = '; expires=' + d.toGMTString();
    }
    return (document.cookie = escape(name) + '=' + escape(value || '') + expire);
  },
  get: function(name) {
    var cookie = document.cookie.match(new RegExp('(^|;)\\s*' + escape(name) + '=([^;\\s]*)'));
    return (cookie ? unescape(cookie[2]) : null);
  },
  erase: function(name) {
    var cookie = Cookie.get(name) || true;
    Cookie.set(name, '', -1);
    return cookie;
  },
  accept: function() {
    if (typeof (navigator.cookieEnabled) == 'boolean') {
      return navigator.cookieEnabled;
    }
    Cookie.set('_test', '1');
    return (Cookie.erase('_test') = '1'); // Line 1807
  }
};

Comment 3 David Kilzer (:ddkilzer) 2007-04-05 11:02:01 PDT
(In reply to comment #2)
> The code that's borken (which may or may not be a legitimate bug):

This is a legitimate bug at first glance.  The code in question parses fine with Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135) but does not with a local debug build of WebKit r20722.  Attaching semi-reduced test case next.

Comment 4 David Kilzer (:ddkilzer) 2007-04-05 11:03:29 PDT
Created attachment 13970 [details]
Test case (partially reduced)

This is a partially reduced (read: copied-and-pasted) test case.  It could probably be reduced further.

Parses fine with Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135), but not with WebKit r20722 thrown into the mix.
Comment 5 David Kilzer (:ddkilzer) 2007-04-05 11:05:30 PDT
(In reply to comment #2)
> The code that's borken (which may or may not be a legitimate bug):
> [...]
>     return (Cookie.erase('_test') = '1'); // Line 1807

Or maybe this a parse error?  Shouldn't "=" be "==" here?

Comment 6 David Kilzer (:ddkilzer) 2007-04-05 11:10:10 PDT
(In reply to comment #5)
> (In reply to comment #2)
> > The code that's borken (which may or may not be a legitimate bug):
> > [...]
> >     return (Cookie.erase('_test') = '1'); // Line 1807
> 
> Or maybe this a parse error?  Shouldn't "=" be "==" here?

Changing "=" to "==" fixed the parsing issue.

Oddly, Firefox 2.0.0.3 allows this statement, although I'm not sure what it's doing (setting the value of an object to '1'?).

I don't have any way to test MSIE 6/7 with this.

Leaving this as a P1 REGRESSION until further analysis can be performed by someone more knowledgeable.  This could easily be an evangelism bug as well.

Comment 7 David Kilzer (:ddkilzer) 2007-04-05 11:15:28 PDT
The code below is NOT a part of Prototype.js 1.4.0.  Apparently it has been added by the site's author, which makes it more likely an evangelism bug.

http://dev.conio.net/repos/prototype/pkg/prototype-1.4.0.tar.gz

(In reply to comment #2)
> The code that's borken (which may or may not be a legitimate bug):
> 
> var Cookie = {
>   set: function(name, value, daysToExpire) {
>     var expire = '';
>     if (daysToExpire != undefined) {
>       var d = new Date();
>       d.setTime(d.getTime() + (86400000 * parseFloat(daysToExpire)));
>       expire = '; expires=' + d.toGMTString();
>     }
>     return (document.cookie = escape(name) + '=' + escape(value || '') +
> expire);
>   },
>   get: function(name) {
>     var cookie = document.cookie.match(new RegExp('(^|;)\\s*' + escape(name) +
> '=([^;\\s]*)'));
>     return (cookie ? unescape(cookie[2]) : null);
>   },
>   erase: function(name) {
>     var cookie = Cookie.get(name) || true;
>     Cookie.set(name, '', -1);
>     return cookie;
>   },
>   accept: function() {
>     if (typeof (navigator.cookieEnabled) == 'boolean') {
>       return navigator.cookieEnabled;
>     }
>     Cookie.set('_test', '1');
>     return (Cookie.erase('_test') = '1'); // Line 1807
>   }
> };
Comment 8 Patagonia 2007-04-06 09:12:27 PDT
Thanks everyone for looking into this.  You're correct, we're running version 1.4 of prototype.js library but the cookie section of it is an "add on".

When looking at this line:

return (Cookie.erase('_test') = '1'); // Line 1807

I agree, it looks like it should be returning a bool, and as such, it looks like it should be a "==" and not "=".  I'll check it out today.
Comment 9 Patagonia 2007-04-06 13:51:04 PDT
I have updated this code on patagonia.com.
Comment 10 David Kilzer (:ddkilzer) 2007-04-06 14:25:22 PDT
(In reply to comment #9)
> I have updated this code on patagonia.com.

Thanks!

Patricia, could you test the site again to see if you still are getting any errors?  Thanks!
Comment 11 David Kilzer (:ddkilzer) 2007-04-06 14:27:06 PDT
(In reply to comment #10)
> Patricia, could you test the site again to see if you still are getting any
> errors?  Thanks!

It looks like the changes haven't been pushed out to the live site quite yet.  Please wait a day or two(?) until the change has been deployed.

Comment 12 Patagonia 2007-04-07 08:32:02 PDT
The change is definitely out there:

http://www.patagonia.com/web/us/js/prototype.js

Try clearing your cache.
Comment 13 Patagonia 2007-04-07 08:35:40 PDT
This was the function I updated, note the last line:

accept: function() {
    if (typeof (navigator.cookieEnabled) == 'boolean') {
      return navigator.cookieEnabled;
    }
    Cookie.set('_test', '1');
    return (Cookie.erase('_test') == '1');
  }

While debugging this I discovered that the bug never surfaced for us because all of our supported browsers offer the "cookieEnabled" property and, hence, never got down to the bottom of the function.  I commented out the 'cookieEnabled' portion of the code to force Firefox to the bottom and was able to reproduce the error you all reported.  It now properly returns a bool.
Comment 14 Patricia Warwick 2007-04-07 16:10:15 PDT
I'm not sure whether the fix is in the version I am using (20766) but I got an error the first time I clicked on the Go button after selecting USA as the country. But when I repeated it  the error did not occur. Here is the message (the dialog box did not appear on top of the window ... I only found it by accident)
 
Something is erroring: 

File Name: Global.js
Function Name: DisplayPopUp
Message: TypeError: Value undefined (result of expression Element.setStyle) is not object., undefined

Refreshing your browser window may fix things.
Comment 15 David Kilzer (:ddkilzer) 2007-04-07 16:56:33 PDT
(In reply to comment #12)
> The change is definitely out there:
> 
> http://www.patagonia.com/web/us/js/prototype.js
> 
> Try clearing your cache.

That file has been corrected.  This file has not (note different URL)!

http://www.patagonia.com/usa/js/prototype.js

This is referenced from this page (which is the one I get when I access http://www.patagonia.com/ directly):

http://www.patagonia.com/web/us/intern_landing.jsp?OPTION=SAR&assetid=15546&target=%2Fhome%2Findex.jsp%3FOPTION%3DHOME_PAGE%26assetid%3D1704
Comment 16 Patagonia 2007-04-07 22:23:36 PDT
Interesting - you will only see that 'usa' directory if you are configured to block all cookies.  I made the fix so that the 'usa' directory now sees the corrected version of the prototype js library.  Thanks.
Comment 17 Robert Blaut 2008-02-14 04:31:07 PST
(In reply to comment #16)
> Interesting - you will only see that 'usa' directory if you are configured to
> block all cookies.  I made the fix so that the 'usa' directory now sees the
> corrected version of the prototype js library.  Thanks.
> 

So it means the bug is already fixed. I checked the page using Webkit r30218 with Cookie enabled and again with disabled. The site works without error.