Closed
Bug 118373
Opened 23 years ago
Closed 20 years ago
escape(null) causes cookies to not be recognized
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: asteele, Unassigned)
References
()
Details
Attachments
(7 files)
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.7) Gecko/20011221 BuildID: 2001122106 The site (www.theissue.org) sets two cookies on the first visit to identify whether the users preference is to have sound played or not. In Netscape 4.7x, 6.2 and IE 6 this works correctly. On second and subsequent visits the browser loads either the with-sound or no-sound site pages based on the last cookie set. With Mozilla 0.9.7 the cookies is not recognised and Mozilla loads the default page which takes the visitor through the initial set-up. The cookie appears to be written into the cookies.txt file OK but is not interpreted by Mozilla. Reproducible: Always Steps to Reproduce: 1.Go to http://www.theissue.org 2.Site loads default page offering sound or no-sound 3.Select no-sound for simplicity 4.Close browser return to the URL and the initial selection pages load again Actual Results: As described... Expected Results: Interpreted the cookie and based on the initial selection by-passed the initial page and gone straight to the no-sound and with-sound default page. I'm happy for any gentle chiding if I have mis-coded the cookie stuff... As mentioned though, it all works OK in IE6, 5.5 and Netscape 4.7x and 6.2
Comment 1•23 years ago
|
||
site is doing: document.cookie = sound + "=" + soundset + "; expires=" + now.toGMTString(); I bet this is bug 118266
Comment 2•23 years ago
|
||
I don't think this is bug 118266, but I could be wrong. My initial test indicated that the cookie.txt file in n4 had this cookie set with a value of "null" (characters "null" actually appear in file) whereas in mozilla the value was the empty string. I suspect therein lies the problem although I was unable to follow-up on this because my browser broke for other reasons. As soon as get my tree fixed, I intend to continue pursuing this.
Comment 3•23 years ago
|
||
Yes, I was right. It's the value of "null" that is causing the problem. I prooved that to myself by forcing the cookie module to return the string "null" for the cookie value when the value itself is the empty string. Attaching a patch to demonstrate that. This is not a proposed patch to fix the problem but rather a proof of concept as to what the problem is. Need to determine if this is an evangelism issue on the site for not recognizing the cookies that it itself set (it indeed set a cookie with no value). Also need to look at the 4.x code and see why/how it forces the value of "null" into the cookie.
Comment 4•23 years ago
|
||
Updated•23 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla0.9.8
Comment 5•23 years ago
|
||
Looked at the code for 4.x. There is nothing in the cookie module itself that forces a value of "null" into a cookie with an empty value. I no longer have a 4.x debug environment so I can't trace where the value of "null" is coming from, but since this is a javascript cookie I would venture a guess that js in 4.x was returning the value of "null" whereas in mozilla it is returning the value of "". So it seems like we have three choices: 1. If a cookie is being set with a value of "", replace that with "null". 2. Same as one but only for javascript cookies 3. Tell site to accept their own cookies having a value of "" Solution 2 is safe in that it won't break existing sites because it exactly mimics the 4.x behavior (if my theory about where the "null" is coming from is correct). However it seems very strange to treat js cookies one way and http cookies another. Solution 1 is uniform at least, but it could break some sites that set http cookies of "".
Comment 6•23 years ago
|
||
Which code exactly is setting the cookie to null?
Comment 7•23 years ago
|
||
jst: Funny you should ask. That's just what I'm investigating right now. Hope to have an answer soon.
Comment 8•23 years ago
|
||
Oops, I didn't mean to put "jst:" on my previous comment. I meant to put "bzbarsky:" there instead.
Comment 9•23 years ago
|
||
OK, I think I know what's happening and I believe it is a js error. Attaching a simplified test case. Consists of two files, theissue.html and welcome.html. Put both files in same directory and bring up theissue.html in the browser. Will explain what's going on after I attach the simplified test case.
Comment 10•23 years ago
|
||
Comment 11•23 years ago
|
||
Comment 12•23 years ago
|
||
Well html files are hard to see in attachments. So I'll also put these two files inline right here. ************ theussue.html ********************* <HTML> <HEAD> <SCRIPT LANGUAGE="JavaScript"> function setCookie(name, value) { dump("setCookie: {"+name+"} = {"+value+"}\n"); document.cookie = name + "=" + escape(value); } function getCookie(name) { var prefix = name + "="; var cookieStartIndex = document.cookie.indexOf(prefix); if (cookieStartIndex == -1) { dump("getCookie: no cookie for {"+name+"} found\n"); return null; } var cookieEndIndex = document.cookie.indexOf(";", cookieStartIndex + prefix.length); if (cookieEndIndex == -1) { cookieEndIndex = document.cookie.length; } var value = document.cookie.substring (cookieStartIndex + prefix.length, cookieEndIndex); dump("getCookie: {"+name+"} = {"+unescape(value)+"}\n"); return unescape(value); } var visits = getCookie("newuser") if (!visits) { dump("site believes that no newuser cookie is set\n"); location = "welcome.html" setCookie("newuser", visits) } </SCRIPT> </HEAD> <BODY> </BODY> </HTML> ***************** welcome.html ************************ <HTML> <BODY> Welcome </BODY> </HTML>
Comment 13•23 years ago
|
||
Hmm.. are we being bitten by our correct support for null DOMStrings? What should null.ToString() be?
Comment 14•23 years ago
|
||
Well I don't yet understand it, but let me post the results of running my simplified test case on both nav4 and on mozilla: 1. Mozilla: First visit to theissue.html: getCookie: no cookie for {newuser} found site believes that no newuser cookie is set setCookie: {newuser} = {null} Subsequent visits to theissue.html: getCookie: {newuser} = {} site believes that no newuser cookie is set setCookie: {newuser} = {} 2. Nav4 First visit to theissue.html: getCookie: no cookie for {newuser} found site believes that no newuser cookie is set setCookie: {newuser} = {null} Subsequent visits to theissue.html: getCookie: no cookie for {newuser} found site believes that no newuser cookie is set setCookie: {newuser} = {null}
Comment 15•23 years ago
|
||
3. IE First visit to theissue.html: getCookie: no cookie for {newuser} found site believes that no newuser cookie is set setCookie: {newuser} = {null} Subsequent visits to theissue.html: getCookie: {newuser} = {null}
Comment 16•23 years ago
|
||
And one more bit of information. If the escape/unescape is removed from the simpified test case, then the results for Mozilla, Nav4, and IE are all identical -- they are the results that was previously obtained for IE.
Comment 17•23 years ago
|
||
OK, here's where the difference lies. In Nav 4 and in IE escape(null) = "null" wheras in mozilla escape(null) = "" Here's a still more simplified test program to demonstrate this: <html> <head> <script> alert("escape(null)={"+escape(null)+"}"); </script> </head> <body> </body> <html>
Comment 18•23 years ago
|
||
And for my final proof, I modified my local copy of the http://www.theissue.com website by removing the escape in the setcookie routine, and everything worked fine. So now I'm convinced that this is not a cookie problem but rather a javascript one per my observation in comment #17. Reassigning. (Now that I understand this better, I retract the three choices I stated in comment #5. None of them were valid.)
Component: Cookies → Javascript Engine
Comment 19•23 years ago
|
||
Reassigning to javascript engine component.
Assignee: morse → rogerl
Status: ASSIGNED → NEW
QA Contact: tever → pschwartau
Comment 20•23 years ago
|
||
Modifying summary from cookies not recognized to escape(null) causes cookies to not be recognized
Summary: Cookies not recognised → escape(null) causes cookies to not be recognized
Comment 21•23 years ago
|
||
Just for the record, I'm attaching three files that make up my local copy of the http://www.the issue.org website. These files are: theissue.html (starting file) welcome.html sound.html page2.html The escape that I spoke about in comment 18 is in the setCookie routine of theissue.html and the identical routine in sound.html. Note that when using these files, make sure to check of the box saying you want sound. That will cause the sound.html file to be loaded in. If you check off the no-sound box, it will attempt to fetch a different file which I have not included here.
Comment 22•23 years ago
|
||
Comment 23•23 years ago
|
||
Comment 24•23 years ago
|
||
Comment 25•23 years ago
|
||
Comment 26•23 years ago
|
||
Sorry, I lied in comment #21. It was four files, not three.
Comment 27•23 years ago
|
||
From Comment #17 above: > OK, here's where the difference lies. In Nav 4 and in IE > escape(null) = "null" > whereas in Mozilla > escape(null) = "" Confirming this behavior. Note that in the browser, the DOM escape() function supersedes the JavaScript Engine escape() function. In fact, in the current standalone JS Engine, we show the Nav4/IE behavior: js> escape(null); null Whereas in a current Mozilla build, the following javascript: URL shows that in the DOM, escape(null)=== the empty string, as Stehen says: javascript: alert(escape(null) === ''); Therefore reassigning to DOM Level 0 component -
Assignee: rogerl → jst
Component: Javascript Engine → DOM Level 0
QA Contact: pschwartau → desale
Comment 28•23 years ago
|
||
Boris was right, this is beause we pass the new null string to nsGlobalWindow::Escape(). I think something as simple as if (isNullDOMString(aStr)) { setDOMStringToNull(aReturn); } would do the job (and would avoid constructing all the encoder stuff we currently build in this case).
Reporter | ||
Comment 29•23 years ago
|
||
As always a most helpful and thorough contribution from everyone. I've asked the author to take note of the comments and adjust the code which has resolved the problem for the particular site. Thus the live site should very shortly function OK. This won't, though, be because the bug has been fixed but because the page code has been changed :-) I hope that the discussion has been helpful for future implementations/development - I've certainly found it instructive.
Comment 30•23 years ago
|
||
Even if the site changes their code to work around the problem, we can still reproduce it using the captured files form the site that I attached in this report.
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Comment 32•22 years ago
|
||
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+, topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword. Please send any questions or feedback about this to adt@netscape.com. You can search for "Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
Updated•22 years ago
|
Target Milestone: mozilla1.2alpha → ---
Comment 34•21 years ago
|
||
OK, so in IE and NS4 escape(null) == "null" (the string, not a null value). Do we want to emulate this behavior?
Comment 35•21 years ago
|
||
The JS spec as the following to stay about escape(): 1. Call ToString(string) 2. Compute the number of characters in Result(1) 3. Let R be the empty string 4. Let k be 0 5. If k equals Result(2), return R So per spec, we are doing the right thing. I searched for some uses of escape(null) on the web and couldn't find any, so I guess if this bug doesn't block the site anymore, we should leave the behavior as is. Just IMHO, of course.
Comment 36•21 years ago
|
||
Um.. What is ToString(null)? Is it ""?
Comment 37•21 years ago
|
||
does it matter? the number of chars in it is 0...
Comment 38•21 years ago
|
||
As long as ToString(null) is not "null" ;) Ok, I'm fine with just marking this wontfix.
Comment 39•21 years ago
|
||
Marking WONTFIX, please feel free to reopen if you disagree.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WONTFIX
Comment 40•21 years ago
|
||
I don't know the EcmaScript spec in enough detail to know what ToString(null) is, but I know JS_ValueToString(null) in Mozilla gives the string "null"...
Comment 41•21 years ago
|
||
Ref: http://www.mozilla.org/js/language/E262-3.pdf I don't know if the EcmaScript spec is relevant here, since in the browser escape() is superseded by the DOM escape() (right?). But FWIW, here is the EcmaScript specification for |escape(null)|: B.2.1 escape(string) When the escape() is called with argument string, the following steps are taken: 1. Call ToString(string) etc. 9.8 ToString The operator ToString converts its argument to a value of type String according to the following table: Input Type Result Undefined "undefined" Null "null" etc. So in pure, browser-independent EcmaScript, escape(null) === "null"
Comment 42•21 years ago
|
||
Reopening, then. Because both IE and NS4 return "null" while we return "".
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 43•20 years ago
|
||
We now use the JS engine's escape(), so this is fixed (by the patch in bug 44272).
Status: REOPENED → RESOLVED
Closed: 21 years ago → 20 years ago
Depends on: 44272
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•