Bug 1342438 - Remove url .hash encoding/decoding prefs r=bagder draft
authorValentin Gosu <valentin.gosu@gmail.com>
Wed, 08 Mar 2017 22:19:34 +0100
changeset 495440 8f2d50d5633496cf165b3925d952bb6475bce3e0
parent 494079 517c553ad64746c479456653ce11b04ab8e4977f
child 548384 b1be067400c53d878155a5cdc6c6b68935c2a1d5
push id48340
push uservalentin.gosu@gmail.com
push dateWed, 08 Mar 2017 21:20:22 +0000
reviewersbagder
bugs1342438
milestone54.0a1
Bug 1342438 - Remove url .hash encoding/decoding prefs r=bagder These prefs have been added close to two years ago: dom.url.encode_decode_hash and dom.url.getters_decode_hash The main reason for their existence was in case we encounter any web-compat issues. At this point the extra code is mostly useless, and flipping the pref may lead to crashes. MozReview-Commit-ID: LhAHkYmv0TR
dom/base/Link.cpp
dom/base/Location.cpp
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/html/test/mochitest.ini
dom/html/test/test_hash_encoded.html
dom/url/URL.cpp
dom/workers/WorkerPrivate.cpp
modules/libpref/init/all.js
netwerk/base/nsStandardURL.cpp
--- a/dom/base/Link.cpp
+++ b/dom/base/Link.cpp
@@ -541,19 +541,16 @@ Link::GetHash(nsAString &_hash)
     // string.
     return;
   }
 
   nsAutoCString ref;
   nsresult rv = uri->GetRef(ref);
   if (NS_SUCCEEDED(rv) && !ref.IsEmpty()) {
     _hash.Assign(char16_t('#'));
-    if (nsContentUtils::GettersDecodeURLHash()) {
-      NS_UnescapeURL(ref); // XXX may result in random non-ASCII bytes!
-    }
     AppendUTF8toUTF16(ref, _hash);
   }
 }
 
 void
 Link::ResetLinkState(bool aNotify, bool aHasHref)
 {
   nsLinkState defaultState;
--- a/dom/base/Location.cpp
+++ b/dom/base/Location.cpp
@@ -303,45 +303,19 @@ Location::GetHash(nsAString& aHash)
     return rv;
   }
 
   nsAutoCString ref;
   nsAutoString unicodeRef;
 
   rv = uri->GetRef(ref);
 
-  if (nsContentUtils::GettersDecodeURLHash()) {
-    if (NS_SUCCEEDED(rv)) {
-      nsCOMPtr<nsITextToSubURI> textToSubURI(
-          do_GetService(NS_ITEXTTOSUBURI_CONTRACTID, &rv));
-
-      if (NS_SUCCEEDED(rv)) {
-        nsAutoCString charset;
-        uri->GetOriginCharset(charset);
-
-        rv = textToSubURI->UnEscapeURIForUI(charset, ref, unicodeRef);
-      }
-
-      if (NS_FAILED(rv)) {
-        // Oh, well.  No intl here!
-        NS_UnescapeURL(ref);
-        CopyASCIItoUTF16(ref, unicodeRef);
-        rv = NS_OK;
-      }
-    }
-
-    if (NS_SUCCEEDED(rv) && !unicodeRef.IsEmpty()) {
-      aHash.Assign(char16_t('#'));
-      aHash.Append(unicodeRef);
-    }
-  } else { // URL Hash should simply return the value of the Ref segment
-    if (NS_SUCCEEDED(rv) && !ref.IsEmpty()) {
-      aHash.Assign(char16_t('#'));
-      AppendUTF8toUTF16(ref, aHash);
-    }
+  if (NS_SUCCEEDED(rv) && !ref.IsEmpty()) {
+    aHash.Assign(char16_t('#'));
+    AppendUTF8toUTF16(ref, aHash);
   }
 
   if (aHash == mCachedHash) {
     // Work around ShareThis stupidly polling location.hash every
     // 5ms all the time by handing out the same exact string buffer
     // we handed out last time.
     aHash = mCachedHash;
   } else {
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -281,18 +281,16 @@ bool nsContentUtils::sIsUnprefixedFullsc
 bool nsContentUtils::sTrustedFullScreenOnly = true;
 bool nsContentUtils::sIsCutCopyAllowed = true;
 bool nsContentUtils::sIsFrameTimingPrefEnabled = false;
 bool nsContentUtils::sIsPerformanceTimingEnabled = false;
 bool nsContentUtils::sIsResourceTimingEnabled = false;
 bool nsContentUtils::sIsUserTimingLoggingEnabled = false;
 bool nsContentUtils::sIsExperimentalAutocompleteEnabled = false;
 bool nsContentUtils::sIsWebComponentsEnabled = false;
-bool nsContentUtils::sEncodeDecodeURLHash = false;
-bool nsContentUtils::sGettersDecodeURLHash = false;
 bool nsContentUtils::sPrivacyResistFingerprinting = false;
 bool nsContentUtils::sSendPerformanceTimingNotifications = false;
 bool nsContentUtils::sUseActivityCursor = false;
 
 uint32_t nsContentUtils::sHandlingInputTimeout = 1000;
 
 uint32_t nsContentUtils::sCookiesLifetimePolicy = nsICookieService::ACCEPT_NORMALLY;
 uint32_t nsContentUtils::sCookiesBehavior = nsICookieService::BEHAVIOR_ACCEPT;
@@ -582,22 +580,16 @@ nsContentUtils::Init()
                                "dom.enable_frame_timing", false);
 
   Preferences::AddBoolVarCache(&sIsExperimentalAutocompleteEnabled,
                                "dom.forms.autocomplete.experimental", false);
 
   Preferences::AddBoolVarCache(&sIsWebComponentsEnabled,
                                "dom.webcomponents.enabled", false);
 
-  Preferences::AddBoolVarCache(&sEncodeDecodeURLHash,
-                               "dom.url.encode_decode_hash", false);
-
-  Preferences::AddBoolVarCache(&sGettersDecodeURLHash,
-                               "dom.url.getters_decode_hash", false);
-
   Preferences::AddBoolVarCache(&sPrivacyResistFingerprinting,
                                "privacy.resistFingerprinting", false);
 
   Preferences::AddUintVarCache(&sHandlingInputTimeout,
                                "dom.event.handling-user-input-time-limit",
                                1000);
 
   Preferences::AddBoolVarCache(&sSendPerformanceTimingNotifications,
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -2104,33 +2104,16 @@ public:
   }
 
   /*
    * Returns true if the frame timing APIs are enabled.
    */
   static bool IsFrameTimingEnabled();
 
   /*
-   * Returns true if URL setters should percent encode the Hash/Ref segment
-   * and getters should return the percent decoded value of the segment
-   */
-  static bool EncodeDecodeURLHash()
-  {
-    return sEncodeDecodeURLHash;
-  }
-
-  /*
-   * Returns true if URL getters should percent decode the value of the segment
-   */
-  static bool GettersDecodeURLHash()
-  {
-    return sGettersDecodeURLHash && sEncodeDecodeURLHash;
-  }
-
-  /*
    * Returns true if the browser should attempt to prevent the given caller type
    * from collecting distinctive information about the browser that could
    * be used to "fingerprint" and track the user across websites.
    */
   static bool ResistFingerprinting(mozilla::dom::CallerType aCallerType)
   {
     return aCallerType != mozilla::dom::CallerType::System &&
            sPrivacyResistFingerprinting;
@@ -2911,18 +2894,16 @@ private:
   static bool sIsCutCopyAllowed;
   static uint32_t sHandlingInputTimeout;
   static bool sIsPerformanceTimingEnabled;
   static bool sIsResourceTimingEnabled;
   static bool sIsUserTimingLoggingEnabled;
   static bool sIsFrameTimingPrefEnabled;
   static bool sIsExperimentalAutocompleteEnabled;
   static bool sIsWebComponentsEnabled;
-  static bool sEncodeDecodeURLHash;
-  static bool sGettersDecodeURLHash;
   static bool sPrivacyResistFingerprinting;
   static bool sSendPerformanceTimingNotifications;
   static bool sUseActivityCursor;
   static uint32_t sCookiesLifetimePolicy;
   static uint32_t sCookiesBehavior;
 
   static nsHtml5StringParser* sHTMLFragmentParser;
   static nsIParser* sXMLFragmentParser;
--- a/dom/html/test/mochitest.ini
+++ b/dom/html/test/mochitest.ini
@@ -578,17 +578,16 @@ skip-if = toolkit == 'android'
 [test_bug741266.html]
 skip-if = toolkit == "android" || toolkit == "windows" # Android: needs control of popup window size, windows(bug 1234520)
 [test_non-ascii-cookie.html]
 support-files = file_cookiemanager.js
 [test_bug765780.html]
 [test_bug871161.html]
 support-files = file_bug871161-1.html file_bug871161-2.html
 [test_bug1013316.html]
-[test_hash_encoded.html]
 [test_bug1081037.html]
 [test_window_open_close.html]
 tags = openwindow
 [test_viewport_resize.html]
 [test_image_clone_load.html]
 [test_bug1203668.html]
 [test_bug1166138.html]
 [test_bug1230665.html]
deleted file mode 100644
--- a/dom/html/test/test_hash_encoded.html
+++ /dev/null
@@ -1,118 +0,0 @@
-<!doctype html>
-<html>
-<head>
-<title>Test link.hash attribute</title>
-<script src="/tests/SimpleTest/SimpleTest.js"></script>
-<link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
-</head>
-<body>
-
-<pre id="test">
-
-<script>
-
-SimpleTest.waitForExplicitFinish();
-SpecialPowers.pushPrefEnv({"set": [['dom.url.encode_decode_hash', false]]}, runTest);
-
-function runTest() {
-  setupTest();
-  doTestEncoded();
-  SimpleTest.finish();
-}
-
-function setupTest() {
-  var target1 = document.createElement("a");
-  target1.id =  "target1";
-  target1.href = "http://www.example.com/#q=♥â¥#hello";
-  document.body.appendChild(target1);
-
-  var target2 = document.createElement("a");
-  target2.id =  "target2";
-  target2.href = "http://www.example.com/#q=%E2%99%A5%C3%A2%C2%A5";
-  document.body.appendChild(target2);
-
-  var target3 = document.createElement("a");
-  target3.id =  "target3";
-  target3.href = "http://www.example.com/#/search/%23important";
-  document.body.appendChild(target3);
-
-  var target4 = document.createElement("a");
-  target4.id =  "target4";
-  target4.href = 'http://www.example.com/#{"a":[13, 42], "b":{"key":"value"}}';
-  document.body.appendChild(target4);
-}
-
-function doTestEncoded() {
-  // Tests Link::GetHash
-
-  // Check that characters aren't being encoded
-  var target = document.getElementById("target1");
-  is(target.hash, '#q=♥â¥#hello', 'Unexpected link hash');
-
-  // Check that encoded characters aren't being decoded
-  target = document.getElementById("target2");
-  is(target.hash, '#q=%E2%99%A5%C3%A2%C2%A5', 'Unexpected link hash');
-
-  // A more regular use case
-  target = document.getElementById("target3");
-  is(target.hash, '#/search/%23important', 'Unexpected link hash');
-
-  // Some JSON
-  target = document.getElementById("target4");
-  is(target.hash, '#{"a":[13, 42], "b":{"key":"value"}}', 'Unexpected link hash');
-
-  // Tests URL::GetHash
-
-  var url = new URL("http://www.example.com/#q=♥â¥#hello")
-  is(url.hash, '#q=♥â¥#hello', 'Unexpected url hash');
-
-  url = new URL("http://www.example.com/#q=%E2%99%A5%C3%A2%C2%A5")
-  is(url.hash, '#q=%E2%99%A5%C3%A2%C2%A5', 'Unexpected url hash');
-
-  url = new URL("http://www.example.com/#/search/%23important")
-  is(url.hash, '#/search/%23important', 'Unexpected url hash');
-
-  // Test getters and setters
-
-  url = new URL("http://www.example.com/");
-  url.hash = "#q=♥â¥#hello%E2%99%A5%C3%A2%C2%A5#/search/%23important"
-  is(url.hash, '#q=♥â¥#hello%E2%99%A5%C3%A2%C2%A5#/search/%23important', 'Unexpected url hash');
-
-  // codepath in nsStandardUrl::SetRef is different if the path is non-empty
-  url = new URL("http://www.example.com/test/");
-  url.hash = "#q=♥â¥#hello%E2%99%A5%C3%A2%C2%A5#/search/%23important"
-  is(url.hash, '#q=♥â¥#hello%E2%99%A5%C3%A2%C2%A5#/search/%23important', 'Unexpected url hash');
-
-  url = new URL("http://www.example.com/");
-  url.hash = '#{"a":[13, 42], "b":{"key":"value"}}';
-  is(target.hash, '#{"a":[13, 42], "b":{"key":"value"}}', 'Unexpected url hash');
-  var parsed = JSON.parse(target.hash.substring(1));
-  is(parsed.b.key, 'value', 'JSON not parsed correctly');
-
-  url = new URL("http://www.example.com/test/");
-  url.hash = '#{"a":[13, 42], "b":{"key":"value"}}';
-  is(target.hash, '#{"a":[13, 42], "b":{"key":"value"}}', 'Unexpected url hash');
-  parsed = JSON.parse(target.hash.substring(1));
-  is(parsed.b.key, 'value', 'JSON not parsed correctly');
-
-  // Tests Location::GetHash
-
-  window.history.pushState(1, document.title, '#q=♥â¥#hello');
-  is(location.hash,'#q=♥â¥#hello', 'Unexpected location hash');
-
-  window.history.pushState(1, document.title, '#q=%E2%99%A5%C3%A2%C2%A5');
-  is(location.hash,'#q=%E2%99%A5%C3%A2%C2%A5', 'Unexpected location hash');
-
-  window.history.pushState(1, document.title, '#/search/%23important');
-  is(location.hash,'#/search/%23important', 'Unexpected location hash');
-
-  window.history.pushState(1, document.title, '#{"a":[13, 42], "b":{"key":"value"}}');
-  is(location.hash,'#{"a":[13, 42], "b":{"key":"value"}}', 'Unexpected location hash');
-
-}
-
-</script>
-
-</pre>
-</body>
-</html>
--- a/dom/url/URL.cpp
+++ b/dom/url/URL.cpp
@@ -556,19 +556,16 @@ void
 URLMainThread::GetHash(nsAString& aHash, ErrorResult& aRv) const
 {
   aHash.Truncate();
 
   nsAutoCString ref;
   nsresult rv = mURI->GetRef(ref);
   if (NS_SUCCEEDED(rv) && !ref.IsEmpty()) {
     aHash.Assign(char16_t('#'));
-    if (nsContentUtils::GettersDecodeURLHash()) {
-      NS_UnescapeURL(ref); // XXX may result in random non-ASCII bytes!
-    }
     AppendUTF8toUTF16(ref, aHash);
   }
 }
 
 void
 URLMainThread::SetHash(const nsAString& aHash, ErrorResult& aRv)
 {
   mURI->SetRef(NS_ConvertUTF16toUTF8(aHash));
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3870,29 +3870,16 @@ WorkerPrivateParent<Derived>::SetBaseURI
   nsCString temp;
 
   if (url && NS_SUCCEEDED(url->GetQuery(temp)) && !temp.IsEmpty()) {
     mLocationInfo.mSearch.Assign('?');
     mLocationInfo.mSearch.Append(temp);
   }
 
   if (NS_SUCCEEDED(aBaseURI->GetRef(temp)) && !temp.IsEmpty()) {
-    nsCOMPtr<nsITextToSubURI> converter =
-      do_GetService(NS_ITEXTTOSUBURI_CONTRACTID);
-    if (converter && nsContentUtils::GettersDecodeURLHash()) {
-      nsCString charset;
-      nsAutoString unicodeRef;
-      if (NS_SUCCEEDED(aBaseURI->GetOriginCharset(charset)) &&
-          NS_SUCCEEDED(converter->UnEscapeURIForUI(charset, temp,
-                                                   unicodeRef))) {
-        mLocationInfo.mHash.Assign('#');
-        mLocationInfo.mHash.Append(NS_ConvertUTF16toUTF8(unicodeRef));
-      }
-    }
-
     if (mLocationInfo.mHash.IsEmpty()) {
       mLocationInfo.mHash.Assign('#');
       mLocationInfo.mHash.Append(temp);
     }
   }
 
   if (NS_SUCCEEDED(aBaseURI->GetScheme(mLocationInfo.mProtocol))) {
     mLocationInfo.mProtocol.Append(':');
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -208,22 +208,16 @@ pref("dom.gamepad.extensions.enabled", f
 // Whether the KeyboardEvent.code is enabled
 pref("dom.keyboardevent.code.enabled", true);
 
 // If this is true, TextEventDispatcher dispatches keydown and keyup events
 // even during composition (keypress events are never fired during composition
 // even if this is true).
 pref("dom.keyboardevent.dispatch_during_composition", false);
 
-// Whether URL,Location,Link::GetHash should be percent encoded
-// in setter and percent decoded in getter (old behaviour = true)
-pref("dom.url.encode_decode_hash", true);
-// Whether ::GetHash should do percent decoding (old behaviour = true)
-pref("dom.url.getters_decode_hash", false);
-
 // Whether to run add-on code in different compartments from browser code. This
 // causes a separate compartment for each (addon, global) combination, which may
 // significantly increase the number of compartments in the system.
 pref("dom.compartment_per_addon", true);
 
 // Fastback caching - if this pref is negative, then we calculate the number
 // of content viewers to cache based on the amount of available memory.
 pref("browser.sessionhistory.max_total_viewers", -1);
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -739,23 +739,18 @@ nsStandardURL::BuildNormalizedSpec(const
         // These next ones *always* add their leading character even if length is 0
         // Handles items like "http://#"
         // ?query
         if (mQuery.mLen >= 0)
             approxLen += 1 + queryEncoder.EncodeSegmentCount(spec, mQuery, esc_Query,        encQuery,     useEncQuery);
         // #ref
 
         if (mRef.mLen >= 0) {
-            if (nsContentUtils::EncodeDecodeURLHash()) {
-                approxLen += 1 + encoder.EncodeSegmentCount(spec, mRef, esc_Ref,
-                                                            encRef, useEncRef);
-            } else {
-                approxLen += 1 + mRef.mLen;
-                useEncRef = false;
-            }
+            approxLen += 1 + encoder.EncodeSegmentCount(spec, mRef, esc_Ref,
+                                                        encRef, useEncRef);
         }
     }
 
     // do not escape the hostname, if IPv6 address literal, mHost will
     // already point to a [ ] delimited IPv6 address literal.
     // However, perform Unicode normalization on it, as IDN does.
     mHostEncoding = eEncoding_ASCII;
     // Note that we don't disallow URLs without a host - file:, etc
@@ -2962,26 +2957,24 @@ nsStandardURL::SetRef(const nsACString &
         ++mPath.mLen;  // Include the # in the path.
         mRef.mPos = mSpec.Length();
         mRef.mLen = 0;
     }
 
     // If precent encoding is necessary, `ref` will point to `buf`'s content.
     // `buf` needs to outlive any use of the `ref` pointer.
     nsAutoCString buf;
-    if (nsContentUtils::EncodeDecodeURLHash()) {
-        // encode ref if necessary
-        bool encoded;
-        GET_SEGMENT_ENCODER(encoder);
-        encoder.EncodeSegmentCount(ref, URLSegment(0, refLen), esc_Ref,
-                                   buf, encoded);
-        if (encoded) {
-            ref = buf.get();
-            refLen = buf.Length();
-        }
+    // encode ref if necessary
+    bool encoded;
+    GET_SEGMENT_ENCODER(encoder);
+    encoder.EncodeSegmentCount(ref, URLSegment(0, refLen), esc_Ref,
+                               buf, encoded);
+    if (encoded) {
+        ref = buf.get();
+        refLen = buf.Length();
     }
 
     int32_t shift = ReplaceSegment(mRef.mPos, mRef.mLen, ref, refLen);
     mPath.mLen += shift;
     mRef.mLen = refLen;
     CALL_RUST_SETTER(SetRef, input);
     return NS_OK;
 }