396389 - [FIX]nsStandardURL needs to serialize mHostEncoding and mSupportsFileURL

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

As things stand, round-tripping a file:// URI through Write/Read gives a URI that is not Equals() to the original one.

Note to self: This patch applies after bug 369566.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

+    // mSpecEncoding and mHostA are just caches that can be recovered as needed.

perhaps there should be an assertion in Read to make sure that they haven't already been set (to values other than eEncoding_Unknown/null)?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 281131 [details] [diff] [review]
Fix

I can add that assertion, sure.

Requesting approval.  This is a fairly safe patch that fixes a bug in URI deserialization that makes file URIs stop being file URIs.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixed, with the assertions added.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 282360 [details] [diff] [review]
testcase

given that the streams are object in/output streams, shouldn't the vars be called objectStream? also, instead of adding 2, objectInStream/objectOutStream seem like better names

+  /*  var uri3 = round_trip(uri1);
+  do_check_true(uri3 instanceof Ci.nsIFileURL);
+  do_check_true(uri1.equals(uri3));
+  */

Why is that commented out?

since you don't have a profile, there's not much point in resetting prefs to their old value... the changes won't be written anyway

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed the names.  Removed the comments from around that code; that was just me testing things.

As for the prefs, I wanted this to not break if it ever moves to a different test framework.

Checked in the test with those changes.