Created attachment 375622 [details] [diff] [review] use SHARD_PATHW (follow up to bug 310071) The code to add a download to the recent documents list got more complicated then it needs to be because MSDN's documentation of SHAddToRecentDocs sucks a bit. This patch should achieve exactly the same.
Looks good, but could you modify test_bug_401430.js to use a non-ASCII filename, and make sure the test passes?
Created attachment 375704 [details] [diff] [review] unit test patch Hm, I've run into a problem here: this test fails for me even without both of my patches and I've got no clue why. However, after I ran the testcase I find a shortcut to testãЛざ124146xxxxxx.doc in my recently used list both with and without the SetState patch, so that looks good. The Windows try server still doesn't run unit tests, right?
The try server does run unit tests - I can push the patch there if you'd like.
You might also try increasing the test's timeout from 1000ms - for some reason SHAddToRecentDocs() doesn't appear to be synchronous :(
(In reply to comment #3) > The try server does run unit tests - I can push the patch there if you'd like. Yes, please. (The page at https://wiki.mozilla.org/Build:TryServer said unit tests are not run.) Also tried it with a 10s timeout -- the new entry was definitely there but the test still failed.
Ah, it might be a problem with how we convert the binary value from the registry into a string, then. I pushed it to the try server, but the test failed: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1241480651.1241489121.16467.gz&fulltext=1#err7 I'll update https://wiki.mozilla.org/Build:TryServer appears to have been updated a while ago: https://wiki.mozilla.org/index.php?title=Build:TryServer&diff=134173&oldid=130790
Yes, that's exactly the same error I got. The value in the registry however is 100% ok. So, after reading http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIWindowsRegKey.idl#269 I see no way to get the raw value of a binary registry value. That leaves only a C++ test and honestly that's too much effort for such little gain.
You can just test for the expanded string, "\xE3\x1b\x04\x56\x30", I guess. It'll get truncated to "\xE3\x1B\x04V0" since it's not valid on it's own, but that matches the value we get from nsIWindowsRegKey, which I think is good enough for now.
Could you combine the two patches, and also move the remove() call for the test file to before the do_check_eq test? This caused trouble in bug 491930. Looks good otherwise, though.
Created attachment 376714 [details] [diff] [review] patch + updated testcase [Checked in: See comment 15 & 16]
Comment on attachment 376714 [details] [diff] [review] patch + updated testcase [Checked in: See comment 15 & 16] I didn't know charCodeAt returned NaN for nonexistent characters! That's kind of handy. r=me, thanks!
Thanks for the review. Do you prefer an additional sr or can I set checkin-needed?
no sr needed, checkin-needed away!
Comment on attachment 376714 [details] [diff] [review] patch + updated testcase [Checked in: See comment 15 & 16] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ed4e2b396665 test part only