Bug 1342438 - Remove url .hash encoding/decoding prefs r=bagder
authorValentin Gosu <valentin.gosu@gmail.com>
Wed, 08 Mar 2017 22:19:34 +0100
changeset 394854 5bd635e6e535e860dfa2858492e1eb38d407a471
parent 394853 5d7591da1a09e235079f0a677eed2e06009ea755
child 394855 7368729b7ba0c213fde567498a71bdaee62d5849
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder
bugs1342438
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -205,22 +205,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
@@ -2978,26 +2973,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;
 }