From 1e019166535279c9cbd3952a160b37cd38038e01 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Thu, 19 Mar 2020 13:03:14 +0000 Subject: [PATCH 01/27] Bug 1531289 - target=_blank with a download should close the download tab, r=nika,Gijs Differential Revision: https://phabricator.services.mozilla.com/D66454 --HG-- extra : moz-landing-system : lando --- browser/base/content/browser.js | 13 ++- browser/base/content/tabbrowser.js | 4 + docshell/base/nsDSURIContentListener.cpp | 63 ++++++++---- docshell/base/nsDSURIContentListener.h | 14 ++- .../mochitest/browser_auto_close_window.js | 96 +++++++++++++++++++ .../exthandler/tests/mochitest/download.sjs | 6 +- .../tests/mochitest/download_page.html | 9 +- 7 files changed, 177 insertions(+), 28 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index e0811713f054..8af9219a482a 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -6206,13 +6206,18 @@ nsBrowserAccess.prototype = { browsingContext = window.content && BrowsingContext.getFromWindow(window.content); if (aURI) { - let loadflags = isExternal - ? Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL - : Ci.nsIWebNavigation.LOAD_FLAGS_NONE; + let loadFlags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE; + if (isExternal) { + loadFlags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL; + } else if (!aTriggeringPrincipal.isSystemPrincipal) { + // XXX this code must be reviewed and changed when bug 1616353 + // lands. + loadFlags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIRST_LOAD; + } gBrowser.loadURI(aURI.spec, { triggeringPrincipal: aTriggeringPrincipal, csp: aCsp, - flags: loadflags, + loadFlags, referrerInfo, }); } diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index 2e3974e43c95..2d58f6d3514c 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -2845,6 +2845,10 @@ } if (fromExternal) { flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL; + } else if (!triggeringPrincipal.isSystemPrincipal) { + // XXX this code must be reviewed and changed when bug 1616353 + // lands. + flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIRST_LOAD; } if (allowMixedContent) { flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ALLOW_MIXED_CONTENT; diff --git a/docshell/base/nsDSURIContentListener.cpp b/docshell/base/nsDSURIContentListener.cpp index 60420ae1bfa2..dee78cef0323 100644 --- a/docshell/base/nsDSURIContentListener.cpp +++ b/docshell/base/nsDSURIContentListener.cpp @@ -10,7 +10,9 @@ #include "nsServiceManagerUtils.h" #include "nsDocShellCID.h" #include "nsIWebNavigationInfo.h" +#include "mozilla/dom/CanonicalBrowsingContext.h" #include "mozilla/dom/Document.h" +#include "mozilla/Unused.h" #include "nsError.h" #include "nsContentSecurityManager.h" #include "nsDocShellLoadTypes.h" @@ -41,25 +43,54 @@ void MaybeCloseWindowHelper::SetShouldCloseWindow(bool aShouldCloseWindow) { } BrowsingContext* MaybeCloseWindowHelper::MaybeCloseWindow() { - if (mShouldCloseWindow) { - // Reset the window context to the opener window so that the dependent - // dialogs have a parent - RefPtr opener = mBrowsingContext->GetOpener(); - - if (opener && !opener->IsDiscarded()) { - mBCToClose = mBrowsingContext; - mBrowsingContext = opener; - - // Now close the old window. Do it on a timer so that we don't run - // into issues trying to close the window before it has fully opened. - NS_ASSERTION(!mTimer, "mTimer was already initialized once!"); - NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, 0, - nsITimer::TYPE_ONE_SHOT); - } + if (!mShouldCloseWindow) { + return mBrowsingContext; } + + // This method should not be called more than once, but it's better to avoid + // closing the current window again. + mShouldCloseWindow = false; + + // Reset the window context to the opener window so that the dependent + // dialogs have a parent + RefPtr newBC = ChooseNewBrowsingContext(mBrowsingContext); + + if (newBC != mBrowsingContext && newBC && !newBC->IsDiscarded()) { + mBCToClose = mBrowsingContext; + mBrowsingContext = newBC; + + // Now close the old window. Do it on a timer so that we don't run + // into issues trying to close the window before it has fully opened. + NS_ASSERTION(!mTimer, "mTimer was already initialized once!"); + NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, 0, + nsITimer::TYPE_ONE_SHOT); + } + return mBrowsingContext; } +already_AddRefed +MaybeCloseWindowHelper::ChooseNewBrowsingContext(BrowsingContext* aBC) { + RefPtr bc = aBC; + + RefPtr opener = bc->GetOpener(); + if (opener && !opener->IsDiscarded()) { + return opener.forget(); + } + + if (!XRE_IsParentProcess()) { + return bc.forget(); + } + + CanonicalBrowsingContext* cbc = CanonicalBrowsingContext::Cast(aBC); + RefPtr wgp = cbc->GetEmbedderWindowGlobal(); + if (!wgp) { + return bc.forget(); + } + + return do_AddRef(wgp->BrowsingContext()); +} + NS_IMETHODIMP MaybeCloseWindowHelper::Notify(nsITimer* timer) { NS_ASSERTION(mBCToClose, "No window to close after timer fired"); @@ -135,7 +166,7 @@ nsDSURIContentListener::DoContent(const nsACString& aContentType, RefPtr maybeCloseWindowHelper = new MaybeCloseWindowHelper(mDocShell->GetBrowsingContext()); maybeCloseWindowHelper->SetShouldCloseWindow(true); - maybeCloseWindowHelper->MaybeCloseWindow(); + Unused << maybeCloseWindowHelper->MaybeCloseWindow(); } return NS_OK; } diff --git a/docshell/base/nsDSURIContentListener.h b/docshell/base/nsDSURIContentListener.h index d0a3b852c2a8..97c8274b3c0a 100644 --- a/docshell/base/nsDSURIContentListener.h +++ b/docshell/base/nsDSURIContentListener.h @@ -17,7 +17,7 @@ class nsIInterfaceRequestor; class nsIWebNavigationInfo; class nsPIDOMWindowOuter; -// Helper Class to eventually close an already openend window +// Helper Class to eventually close an already opened window class MaybeCloseWindowHelper final : public nsITimerCallback { public: NS_DECL_ISUPPORTS @@ -27,9 +27,12 @@ class MaybeCloseWindowHelper final : public nsITimerCallback { mozilla::dom::BrowsingContext* aContentContext); /** - * Closes the provided window async (if mShouldCloseWindow is true) - * and returns its opener if the window was just opened. Otherwise - * returns the BrowsingContext provided in the constructor. + * Closes the provided window async (if mShouldCloseWindow is true) and + * returns a valid browsingContext to be used instead as parent for dialogs or + * similar things. + * In case mShouldCloseWindow is true, the final browsing context will be the + * a valid new chrome window to use. It can be the opener, or the opener's + * top, or the top chrome window. */ mozilla::dom::BrowsingContext* MaybeCloseWindow(); @@ -39,6 +42,9 @@ class MaybeCloseWindowHelper final : public nsITimerCallback { ~MaybeCloseWindowHelper(); private: + already_AddRefed ChooseNewBrowsingContext( + mozilla::dom::BrowsingContext* aBC); + /** * The dom window associated to handle content. */ diff --git a/uriloader/exthandler/tests/mochitest/browser_auto_close_window.js b/uriloader/exthandler/tests/mochitest/browser_auto_close_window.js index fe898837ff9d..6b988f628e39 100644 --- a/uriloader/exthandler/tests/mochitest/browser_auto_close_window.js +++ b/uriloader/exthandler/tests/mochitest/browser_auto_close_window.js @@ -100,6 +100,62 @@ add_task(async function target_blank() { }); }); +add_task(async function target_blank_no_opener() { + // Tests that a link with target=_blank and no opener opens a new tab + // and closes it, returning the window that we're using for navigation. + await BrowserTestUtils.withNewTab({ gBrowser, url: PAGE_URL }, async function( + browser + ) { + let dialogAppeared = promiseHelperAppDialog(); + let tabOpened = BrowserTestUtils.waitForEvent( + gBrowser.tabContainer, + "TabOpen" + ).then(event => { + return [event.target, BrowserTestUtils.waitForTabClosing(event.target)]; + }); + + await BrowserTestUtils.synthesizeMouseAtCenter( + "#target_blank_no_opener", + {}, + browser + ); + + let windowContext = await dialogAppeared; + is(windowContext, browser.ownerGlobal, "got the right windowContext"); + let [tab, closingPromise] = await tabOpened; + await closingPromise; + is(tab.linkedBrowser, null, "tab was opened and closed"); + }); +}); + +add_task(async function open_in_new_tab_no_opener() { + // Tests that a link with target=_blank and no opener opens a new tab + // and closes it, returning the window that we're using for navigation. + await BrowserTestUtils.withNewTab({ gBrowser, url: PAGE_URL }, async function( + browser + ) { + let dialogAppeared = promiseHelperAppDialog(); + let tabOpened = BrowserTestUtils.waitForEvent( + gBrowser.tabContainer, + "TabOpen" + ).then(event => { + return [event.target, BrowserTestUtils.waitForTabClosing(event.target)]; + }); + + await BrowserTestUtils.synthesizeMouseAtCenter( + "#open_in_new_tab_no_opener", + {}, + browser + ); + + let windowContext = await dialogAppeared; + is(windowContext, browser.ownerGlobal, "got the right windowContext"); + let [tab, closingPromise] = await tabOpened; + await closingPromise; + is(tab.linkedBrowser, null, "tab was opened and closed"); + }); +}); + add_task(async function new_window() { // Tests that a link that forces us to open a new window (by specifying a // width and a height in window.open) opens a new window for the load, @@ -122,6 +178,46 @@ add_task(async function new_window() { // The window should close on its own. If not, this test will time out. await BrowserTestUtils.domWindowClosed(win); ok(win.closed, "window was opened and closed"); + + is( + await fetch(SJS_URL + "?reset").then(r => r.text()), + "OK", + "Test reseted" + ); + }); +}); + +add_task(async function new_window_no_opener() { + // Tests that a link that forces us to open a new window (by specifying a + // width and a height in window.open) opens a new window for the load, + // realizes that we need to close that window and returns the *original* + // window as the window context. + await BrowserTestUtils.withNewTab({ gBrowser, url: PAGE_URL }, async function( + browser + ) { + let dialogAppeared = promiseHelperAppDialog(); + let windowOpened = BrowserTestUtils.waitForNewWindow(); + + await BrowserTestUtils.synthesizeMouseAtCenter( + "#new_window_no_opener", + {}, + browser + ); + let win = await windowOpened; + // Now allow request to complete: + fetch(SJS_URL + "?finish"); + + await dialogAppeared; + + // The window should close on its own. If not, this test will time out. + await BrowserTestUtils.domWindowClosed(win); + ok(win.closed, "window was opened and closed"); + + is( + await fetch(SJS_URL + "?reset").then(r => r.text()), + "OK", + "Test reseted" + ); }); }); diff --git a/uriloader/exthandler/tests/mochitest/download.sjs b/uriloader/exthandler/tests/mochitest/download.sjs index cd8e426dde0d..bee7bd70154c 100644 --- a/uriloader/exthandler/tests/mochitest/download.sjs +++ b/uriloader/exthandler/tests/mochitest/download.sjs @@ -20,9 +20,13 @@ function handleRequest(req, res) { // Set a variable to allow the request to complete immediately: setState("finishReq", "true"); } + } else if (req.queryString.includes('reset')) { + res.write("OK"); + setObjectState("downloadReq", null); + setState("finishReq", "false"); } else { res.processAsync(); - if (getState("finishReq")) { + if (getState("finishReq") === "true") { actuallyHandleRequest(req, res); } else { let o = {callback() { actuallyHandleRequest(req, res) }}; diff --git a/uriloader/exthandler/tests/mochitest/download_page.html b/uriloader/exthandler/tests/mochitest/download_page.html index 1cb331aacf3d..5a264888fa77 100644 --- a/uriloader/exthandler/tests/mochitest/download_page.html +++ b/uriloader/exthandler/tests/mochitest/download_page.html @@ -6,14 +6,17 @@ Test page for link clicking regular load target blank - new window + new window click to reopen + target blank (noopener) + click to reopen (noopener) + new window (noopener) From e4db0e4496ef765e035da5fa18d92fb9bc6e5340 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Thu, 19 Mar 2020 13:06:48 +0000 Subject: [PATCH 02/27] Bug 1220810 - Hardcode localhost to loopback, r=ckerschb,dragana Differential Revision: https://phabricator.services.mozilla.com/D64586 --HG-- extra : moz-landing-system : lando --- dom/base/test/unit/test_error_codes.js | 3 + dom/media/tests/mochitest/mochitest.ini | 2 + dom/performance/PerformanceTiming.cpp | 12 + dom/security/nsMixedContentBlocker.cpp | 8 +- dom/security/nsMixedContentBlocker.h | 5 + dom/security/test/gtest/TestSecureContext.cpp | 24 +- .../test_isOriginPotentiallyTrustworthy.js | 1 + .../browser/browser_webauthn_ipaddress.js | 1 + modules/libpref/init/StaticPrefList.yaml | 6 + modules/libpref/init/all.js | 2 - netwerk/base/nsProtocolProxyService.cpp | 17 +- netwerk/base/nsProtocolProxyService.h | 1 - netwerk/dns/DNS.cpp | 47 +- netwerk/dns/DNS.h | 4 + netwerk/dns/nsHostResolver.cpp | 540 ++++++++++-------- netwerk/dns/nsHostResolver.h | 8 + netwerk/test/unit/test_about_networking.js | 4 + netwerk/test/unit/test_dns_offline.js | 3 + .../test/unit/test_dns_originAttributes.js | 5 + .../test/unit/test_ping_aboutnetworking.js | 2 + netwerk/test/unit/test_trr.js | 2 + 21 files changed, 421 insertions(+), 276 deletions(-) diff --git a/dom/base/test/unit/test_error_codes.js b/dom/base/test/unit/test_error_codes.js index a06ebfd9aace..2a963d6aa5e6 100644 --- a/dom/base/test/unit/test_error_codes.js +++ b/dom/base/test/unit/test_error_codes.js @@ -38,6 +38,8 @@ function run_test_pt1() { } catch (e) {} Services.io.offline = true; prefs.setBoolPref("network.dns.offline-localhost", false); + // We always resolve localhost as it's hardcoded without the following pref: + prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); gExpectedStatus = Cr.NS_ERROR_OFFLINE; gNextTestFunc = run_test_pt2; @@ -49,6 +51,7 @@ function run_test_pt1() { function run_test_pt2() { Services.io.offline = false; prefs.clearUserPref("network.dns.offline-localhost"); + prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); gExpectedStatus = Cr.NS_ERROR_CONNECTION_REFUSED; gNextTestFunc = end_test; diff --git a/dom/media/tests/mochitest/mochitest.ini b/dom/media/tests/mochitest/mochitest.ini index 4dd01c8bb29b..428ae46a54f1 100644 --- a/dom/media/tests/mochitest/mochitest.ini +++ b/dom/media/tests/mochitest/mochitest.ini @@ -2,6 +2,8 @@ tags = mtg webrtc subsuite = media scheme = https +prefs = + network.proxy.allow_hijacking_localhost=true support-files = head.js dataChannel.js diff --git a/dom/performance/PerformanceTiming.cpp b/dom/performance/PerformanceTiming.cpp index 30694e769f87..43a4a1f49963 100644 --- a/dom/performance/PerformanceTiming.cpp +++ b/dom/performance/PerformanceTiming.cpp @@ -87,6 +87,18 @@ PerformanceTiming::PerformanceTiming(Performance* aPerformance, : nsRFPService::ReduceTimePrecisionAsMSecs( aZeroTime, aPerformance->GetRandomTimelineSeed()))); +#ifdef DEBUG + if (mTimingData->ResponseStartHighRes(aPerformance) - + mTimingData->ZeroTime() < + 0) { + MOZ_CRASH_UNSAFE_PRINTF( + "Heisenbug Reproduced: Please file line in 1436778. %s %f - %f (%f)", + (aPerformance->IsSystemPrincipal() ? "System" : "Not-System"), + mTimingData->ResponseStartHighRes(aPerformance), + mTimingData->ZeroTime(), aZeroTime); + } +#endif + // Non-null aHttpChannel implies that this PerformanceTiming object is being // used for subresources, which is irrelevant to this probe. if (!aHttpChannel && StaticPrefs::dom_enable_performance() && diff --git a/dom/security/nsMixedContentBlocker.cpp b/dom/security/nsMixedContentBlocker.cpp index c4b67750f47f..d49a812c28ff 100644 --- a/dom/security/nsMixedContentBlocker.cpp +++ b/dom/security/nsMixedContentBlocker.cpp @@ -379,8 +379,7 @@ nsMixedContentBlocker::ShouldLoad(nsIURI* aContentLocation, bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost( const nsACString& aAsciiHost) { - if (aAsciiHost.EqualsLiteral("::1") || - aAsciiHost.EqualsLiteral("localhost")) { + if (mozilla::net::IsLoopbackHostname(aAsciiHost)) { return true; } @@ -400,9 +399,8 @@ bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost( // https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy says // we should only consider [::1]/128 as a potentially trustworthy IPv6 // address, whereas for IPv4 127.0.0.1/8 are considered as potentially - // trustworthy. We already handled "[::1]" above, so all that's remained to - // handle here are IPv4 loopback addresses. - return IsIPAddrV4(&addr) && IsLoopBackAddress(&addr); + // trustworthy. + return IsLoopBackAddressWithoutIPv6Mapping(&addr); } bool nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackURL(nsIURI* aURL) { diff --git a/dom/security/nsMixedContentBlocker.h b/dom/security/nsMixedContentBlocker.h index 9997a4cf2a24..54e97fe6607a 100644 --- a/dom/security/nsMixedContentBlocker.h +++ b/dom/security/nsMixedContentBlocker.h @@ -34,6 +34,11 @@ enum MixedContentTypes { using mozilla::OriginAttributes; class nsILoadInfo; // forward declaration +namespace mozilla { +namespace net { +class nsProtocolProxyService; // forward declaration +} +} // namespace mozilla class nsMixedContentBlocker : public nsIContentPolicy, public nsIChannelEventSink { diff --git a/dom/security/test/gtest/TestSecureContext.cpp b/dom/security/test/gtest/TestSecureContext.cpp index f971963d9e7d..3156139fa064 100644 --- a/dom/security/test/gtest/TestSecureContext.cpp +++ b/dom/security/test/gtest/TestSecureContext.cpp @@ -24,12 +24,29 @@ struct TestExpectations { bool expectedResult; }; +class MOZ_RAII AutoRestoreBoolPref final { + public: + AutoRestoreBoolPref(const char* aPref, bool aValue) : mPref(aPref) { + Preferences::GetBool(mPref, &mOldValue); + Preferences::SetBool(mPref, aValue); + } + + ~AutoRestoreBoolPref() { Preferences::SetBool(mPref, mOldValue); } + + private: + const char* mPref = nullptr; + bool mOldValue = false; +}; + // ============================= TestDirectives ======================== TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) { // boolean isOriginPotentiallyTrustworthy(in nsIPrincipal aPrincipal); + AutoRestoreBoolPref savedPref("network.proxy.allow_hijacking_localhost", + false); + static const TestExpectations uris[] = { {"http://example.com/", false}, {"https://example.com/", true}, @@ -39,7 +56,9 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) {"ftp://example.com", false}, {"about:config", false}, {"http://localhost", true}, - {"http://xyzzy.localhost", false}, + {"http://localhost.localhost", true}, + {"http://a.b.c.d.e.localhost", true}, + {"http://xyzzy.localhost", true}, {"http://127.0.0.1", true}, {"http://127.0.0.2", true}, {"http://127.1.0.1", true}, @@ -72,7 +91,8 @@ TEST(SecureContext, IsOriginPotentiallyTrustworthyWithContentPrincipal) bool isPotentiallyTrustworthy = false; rv = prin->GetIsOriginPotentiallyTrustworthy(&isPotentiallyTrustworthy); ASSERT_EQ(NS_OK, rv); - ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult); + ASSERT_EQ(isPotentiallyTrustworthy, uris[i].expectedResult) + << uris[i].uri << uris[i].expectedResult; } } diff --git a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js index 427543a108a3..9286470f82ab 100644 --- a/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js +++ b/dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js @@ -36,6 +36,7 @@ add_task(async function test_isOriginPotentiallyTrustworthy() { ["http://example.com/", false], ["https://example.com/", true], ["http://localhost/", true], + ["http://localhost.localhost/", true], ["http://127.0.0.1/", true], ["file:///", true], ["resource:///", true], diff --git a/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js b/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js index c19e372e37d6..a7d85ba8ac59 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js +++ b/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js @@ -21,6 +21,7 @@ add_task(function test_setup() { "security.webauth.webauthn_enable_usbtoken", false ); + Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); }); registerCleanupFunction(async function() { diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index bc9cc3075498..9b0b31dd8eee 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -7231,6 +7231,12 @@ value: true mirror: always +# Set true to allow resolving proxy for localhost +- name: network.proxy.allow_hijacking_localhost + type: RelaxedAtomicBool + value: false + mirror: always + # Allow CookieJarSettings to be unblocked for channels without a document. # This is for testing only. - name: network.cookieJarSettings.unblocked_for_testing diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index b911c1200c89..c5d79e333ca5 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1978,8 +1978,6 @@ pref("network.proxy.socks_port", 0); pref("network.proxy.socks_version", 5); pref("network.proxy.proxy_over_tls", true); pref("network.proxy.no_proxies_on", ""); -// Set true to allow resolving proxy for localhost -pref("network.proxy.allow_hijacking_localhost", false); pref("network.proxy.failover_timeout", 1800); // 30 minutes pref("network.online", true); //online/offline diff --git a/netwerk/base/nsProtocolProxyService.cpp b/netwerk/base/nsProtocolProxyService.cpp index f163d522a98a..97961eaac47f 100644 --- a/netwerk/base/nsProtocolProxyService.cpp +++ b/netwerk/base/nsProtocolProxyService.cpp @@ -36,7 +36,9 @@ #include "nsISystemProxySettings.h" #include "nsINetworkLinkService.h" #include "nsIHttpChannelInternal.h" +#include "mozilla/dom/nsMixedContentBlocker.h" #include "mozilla/Logging.h" +#include "mozilla/StaticPrefs_network.h" #include "mozilla/Tokenizer.h" #include "mozilla/Unused.h" @@ -770,7 +772,6 @@ nsProtocolProxyService::nsProtocolProxyService() mSOCKSProxyRemoteDNS(false), mProxyOverTLS(true), mWPADOverDHCPEnabled(false), - mAllowHijackingLocalhost(false), mPACMan(nullptr), mSessionStart(PR_Now()), mFailedProxyTimeout(30 * 60) // 30 minute default @@ -1018,11 +1019,6 @@ void nsProtocolProxyService::PrefsChanged(nsIPrefBranch* prefBranch, reloadPAC = reloadPAC || mProxyConfig == PROXYCONFIG_WPAD; } - if (!pref || !strcmp(pref, PROXY_PREF("allow_hijacking_localhost"))) { - proxy_GetBoolPref(prefBranch, PROXY_PREF("allow_hijacking_localhost"), - mAllowHijackingLocalhost); - } - if (!pref || !strcmp(pref, PROXY_PREF("failover_timeout"))) proxy_GetIntPref(prefBranch, PROXY_PREF("failover_timeout"), mFailedProxyTimeout); @@ -1096,9 +1092,12 @@ bool nsProtocolProxyService::CanUseProxy(nsIURI* aURI, int32_t defaultPort) { // Don't use proxy for local hosts (plain hostname, no dots) if ((!is_ipaddr && mFilterLocalHosts && !host.Contains('.')) || - (!mAllowHijackingLocalhost && - (host.EqualsLiteral("127.0.0.1") || host.EqualsLiteral("::1") || - host.EqualsLiteral("localhost")))) { + // This method detects if we have network.proxy.allow_hijacking_localhost + // pref enabled. If it's true then this method will always return false + // otherwise it returns true if the host matches an address that's + // hardcoded to the loopback address. + (!StaticPrefs::network_proxy_allow_hijacking_localhost() && + nsMixedContentBlocker::IsPotentiallyTrustworthyLoopbackHost(host))) { LOG(("Not using proxy for this local host [%s]!\n", host.get())); return false; // don't allow proxying } diff --git a/netwerk/base/nsProtocolProxyService.h b/netwerk/base/nsProtocolProxyService.h index feab7132a7a7..e52c0d34600b 100644 --- a/netwerk/base/nsProtocolProxyService.h +++ b/netwerk/base/nsProtocolProxyService.h @@ -387,7 +387,6 @@ class nsProtocolProxyService final : public nsIProtocolProxyService2, bool mSOCKSProxyRemoteDNS; bool mProxyOverTLS; bool mWPADOverDHCPEnabled; - bool mAllowHijackingLocalhost; RefPtr mPACMan; // non-null if we are using PAC nsCOMPtr mSystemProxySettings; diff --git a/netwerk/dns/DNS.cpp b/netwerk/dns/DNS.cpp index fad9d4f48abb..0c2126c8e7f6 100644 --- a/netwerk/dns/DNS.cpp +++ b/netwerk/dns/DNS.cpp @@ -6,9 +6,11 @@ #include "mozilla/net/DNS.h" +#include "mozilla/ArrayUtils.h" #include "mozilla/Assertions.h" #include "mozilla/mozalloc.h" -#include "mozilla/ArrayUtils.h" +#include "mozilla/StaticPrefs_network.h" +#include "nsContentUtils.h" #include "nsString.h" #include @@ -139,21 +141,46 @@ bool NetAddrToString(const NetAddr* addr, char* buf, uint32_t bufSize) { } bool IsLoopBackAddress(const NetAddr* addr) { + if (IsLoopBackAddressWithoutIPv6Mapping(addr)) { + return true; + } + if (addr->raw.family != AF_INET6) { + return false; + } + + return IPv6ADDR_IS_V4MAPPED(&addr->inet6.ip) && + IPv6ADDR_V4MAPPED_TO_IPADDR(&addr->inet6.ip) == htonl(INADDR_LOOPBACK); +} + +bool IsLoopBackAddressWithoutIPv6Mapping(const NetAddr* addr) { if (addr->raw.family == AF_INET) { // Consider 127.0.0.1/8 as loopback uint32_t ipv4Addr = ntohl(addr->inet.ip); return (ipv4Addr >> 24) == 127; } - if (addr->raw.family == AF_INET6) { - if (IPv6ADDR_IS_LOOPBACK(&addr->inet6.ip)) { - return true; - } - if (IPv6ADDR_IS_V4MAPPED(&addr->inet6.ip) && - IPv6ADDR_V4MAPPED_TO_IPADDR(&addr->inet6.ip) == - htonl(INADDR_LOOPBACK)) { - return true; - } + + if (addr->raw.family == AF_INET6 && IPv6ADDR_IS_LOOPBACK(&addr->inet6.ip)) { + return true; } + + return false; +} + +bool IsLoopbackHostname(const nsACString& aAsciiHost) { + // If the user has configured to proxy localhost addresses don't consider them + // to be secure + if (StaticPrefs::network_proxy_allow_hijacking_localhost()) { + return false; + } + + nsAutoCString host; + nsContentUtils::ASCIIToLower(aAsciiHost, host); + + if (host.EqualsLiteral("localhost") || + StringEndsWith(host, NS_LITERAL_CSTRING(".localhost"))) { + return true; + } + return false; } diff --git a/netwerk/dns/DNS.h b/netwerk/dns/DNS.h index b81b5e44fdd8..0867ec8099ca 100644 --- a/netwerk/dns/DNS.h +++ b/netwerk/dns/DNS.h @@ -182,6 +182,10 @@ bool NetAddrToString(const NetAddr* addr, char* buf, uint32_t bufSize); bool IsLoopBackAddress(const NetAddr* addr); +bool IsLoopBackAddressWithoutIPv6Mapping(const NetAddr* addr); + +bool IsLoopbackHostname(const nsACString& aAsciiHost); + bool IsIPAddrAny(const NetAddr* addr); bool IsIPAddrV4(const NetAddr* addr); diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 7edba72f429f..98f0f241436f 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -784,11 +784,7 @@ nsresult nsHostResolver::GetHostRecord(const nsACString& host, RefPtr& entry = mRecordDB.GetOrInsert(key); if (!entry) { - if (IS_ADDR_TYPE(type)) { - entry = new AddrHostRecord(key); - } else { - entry = new TypeHostRecord(key); - } + entry = InitRecord(key); } RefPtr rec = entry; @@ -807,6 +803,48 @@ nsresult nsHostResolver::GetHostRecord(const nsACString& host, return NS_OK; } +nsHostRecord* nsHostResolver::InitRecord(const nsHostKey& key) { + if (IS_ADDR_TYPE(key.type)) { + return new AddrHostRecord(key); + } + return new TypeHostRecord(key); +} + +already_AddRefed nsHostResolver::InitLoopbackRecord( + const nsHostKey& key, nsresult* aRv) { + MOZ_ASSERT(aRv); + MOZ_ASSERT(IS_ADDR_TYPE(key.type)); + + *aRv = NS_ERROR_FAILURE; + RefPtr rec = InitRecord(key); + + RefPtr addrRec = do_QueryObject(rec); + MutexAutoLock lock(addrRec->addr_info_lock); + + PRNetAddr prAddr; + + if (key.af == PR_AF_INET) { + MOZ_RELEASE_ASSERT(PR_StringToNetAddr("127.0.0.1", &prAddr) == PR_SUCCESS); + } else { + MOZ_RELEASE_ASSERT(PR_StringToNetAddr("::1", &prAddr) == PR_SUCCESS); + } + + RefPtr ai; + *aRv = GetAddrInfo(rec->host, rec->af, addrRec->flags, getter_AddRefs(ai), + addrRec->mGetTtl); + if (NS_WARN_IF(NS_FAILED(*aRv))) { + return nullptr; + } + + addrRec->addr_info = ai; + addrRec->SetExpiration(TimeStamp::NowLoRes(), mDefaultCacheLifetime, + mDefaultGracePeriod); + addrRec->negative = false; + + *aRv = NS_OK; + return rec.forget(); +} + nsresult nsHostResolver::ResolveHost(const nsACString& aHost, const nsACString& aTrrServer, uint16_t type, @@ -852,268 +890,276 @@ nsresult nsHostResolver::ResolveHost(const nsACString& aHost, MutexAutoLock lock(mLock); if (mShutdown) { - rv = NS_ERROR_NOT_INITIALIZED; - } else { - // check to see if there is already an entry for this |host| - // in the hash table. if so, then check to see if we can't - // just reuse the lookup result. otherwise, if there are - // any pending callbacks, then add to pending callbacks queue, - // and return. otherwise, add ourselves as first pending - // callback, and proceed to do the lookup. - nsAutoCString originSuffix; - aOriginAttributes.CreateSuffix(originSuffix); + return NS_ERROR_NOT_INITIALIZED; + } - if (gTRRService && gTRRService->IsExcludedFromTRR(host)) { - flags |= RES_DISABLE_TRR; + // check to see if there is already an entry for this |host| + // in the hash table. if so, then check to see if we can't + // just reuse the lookup result. otherwise, if there are + // any pending callbacks, then add to pending callbacks queue, + // and return. otherwise, add ourselves as first pending + // callback, and proceed to do the lookup. + nsAutoCString originSuffix; + aOriginAttributes.CreateSuffix(originSuffix); - if (!aTrrServer.IsEmpty()) { - return NS_ERROR_UNKNOWN_HOST; - } + if (gTRRService && gTRRService->IsExcludedFromTRR(host)) { + flags |= RES_DISABLE_TRR; + + if (!aTrrServer.IsEmpty()) { + return NS_ERROR_UNKNOWN_HOST; + } + } + + nsHostKey key(host, aTrrServer, type, flags, af, + (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix); + + // Check if we have a localhost domain, if so hardcode to loopback + if (IS_ADDR_TYPE(type) && IsLoopbackHostname(host)) { + nsresult rv; + RefPtr result = InitLoopbackRecord(key, &rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + MOZ_ASSERT(result); + aCallback->OnResolveHostComplete(this, result, NS_OK); + return NS_OK; + } + + RefPtr& entry = mRecordDB.GetOrInsert(key); + if (!entry) { + entry = InitRecord(key); + } + + RefPtr rec = entry; + RefPtr addrRec = do_QueryObject(rec); + MOZ_ASSERT(rec, "Record should not be null"); + MOZ_ASSERT((IS_ADDR_TYPE(type) && rec->IsAddrRecord() && addrRec) || + (IS_OTHER_TYPE(type) && !rec->IsAddrRecord())); + + if (!(flags & RES_BYPASS_CACHE) && + rec->HasUsableResult(TimeStamp::NowLoRes(), flags)) { + LOG((" Using cached record for host [%s].\n", host.get())); + // put reference to host record on stack... + result = rec; + if (IS_ADDR_TYPE(type)) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); } - nsHostKey key(host, aTrrServer, type, flags, af, - (aOriginAttributes.mPrivateBrowsingId > 0), originSuffix); - RefPtr& entry = mRecordDB.GetOrInsert(key); - if (!entry) { + // For entries that are in the grace period + // or all cached negative entries, use the cache but start a new + // lookup in the background + ConditionallyRefreshRecord(rec, host); + + if (rec->negative) { + LOG((" Negative cache entry for host [%s].\n", host.get())); if (IS_ADDR_TYPE(type)) { - entry = new AddrHostRecord(key); - } else { - entry = new TypeHostRecord(key); + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, + METHOD_NEGATIVE_HIT); } + status = NS_ERROR_UNKNOWN_HOST; } - RefPtr rec = entry; - RefPtr addrRec = do_QueryObject(rec); - MOZ_ASSERT(rec, "Record should not be null"); - MOZ_ASSERT((IS_ADDR_TYPE(type) && rec->IsAddrRecord() && addrRec) || - (IS_OTHER_TYPE(type) && !rec->IsAddrRecord())); + // Check whether host is a IP address for A/AAAA queries. + // For by-type records we have already checked at the beginning of + // this function. + } else if (addrRec && addrRec->addr) { + // if the host name is an IP address literal and has been + // parsed, go ahead and use it. + LOG((" Using cached address for IP Literal [%s].\n", host.get())); + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); + result = rec; + } else if (addrRec && + PR_StringToNetAddr(host.get(), &tempAddr) == PR_SUCCESS) { + // try parsing the host name as an IP address literal to short + // circuit full host resolution. (this is necessary on some + // platforms like Win9x. see bug 219376 for more details.) + LOG((" Host is IP Literal [%s].\n", host.get())); - // Check if the entry is vaild. - if (!(flags & RES_BYPASS_CACHE) && - rec->HasUsableResult(TimeStamp::NowLoRes(), flags)) { - LOG((" Using cached record for host [%s].\n", host.get())); - // put reference to host record on stack... - result = rec; - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - } + // ok, just copy the result into the host record, and be + // done with it! ;-) + addrRec->addr = MakeUnique(); + PRNetAddrToNetAddr(&tempAddr, addrRec->addr.get()); + // put reference to host record on stack... + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); + result = rec; - // For entries that are in the grace period - // or all cached negative entries, use the cache but start a new - // lookup in the background - ConditionallyRefreshRecord(rec, host); + // Check if we have received too many requests. + } else if (mPendingCount >= MAX_NON_PRIORITY_REQUESTS && + !IsHighPriority(flags) && !rec->mResolving) { + LOG( + (" Lookup queue full: dropping %s priority request for " + "host [%s].\n", + IsMediumPriority(flags) ? "medium" : "low", host.get())); + if (IS_ADDR_TYPE(type)) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_OVERFLOW); + } + // This is a lower priority request and we are swamped, so refuse it. + rv = NS_ERROR_DNS_LOOKUP_QUEUE_FULL; - if (rec->negative) { - LOG((" Negative cache entry for host [%s].\n", host.get())); - if (IS_ADDR_TYPE(type)) { + // Check if the offline flag is set. + } else if (flags & RES_OFFLINE) { + LOG((" Offline request for host [%s]; ignoring.\n", host.get())); + rv = NS_ERROR_OFFLINE; + + // We do not have a valid result till here. + // A/AAAA request can check for an alternative entry like AF_UNSPEC. + // Otherwise we need to start a new query. + } else if (!rec->mResolving) { + // If this is an IPV4 or IPV6 specific request, check if there is + // an AF_UNSPEC entry we can use. Otherwise, hit the resolver... + if (addrRec && !(flags & RES_BYPASS_CACHE) && + ((af == PR_AF_INET) || (af == PR_AF_INET6))) { + // Check for an AF_UNSPEC entry. + + const nsHostKey unspecKey( + host, aTrrServer, nsIDNSService::RESOLVE_TYPE_DEFAULT, flags, + PR_AF_UNSPEC, (aOriginAttributes.mPrivateBrowsingId > 0), + originSuffix); + RefPtr unspecRec = mRecordDB.Get(unspecKey); + + TimeStamp now = TimeStamp::NowLoRes(); + if (unspecRec && unspecRec->HasUsableResult(now, flags)) { + MOZ_ASSERT(unspecRec->IsAddrRecord()); + + RefPtr addrUnspecRec = do_QueryObject(unspecRec); + MOZ_ASSERT(addrUnspecRec); + MOZ_ASSERT(addrUnspecRec->addr_info || addrUnspecRec->negative, + "Entry should be resolved or negative."); + + LOG((" Trying AF_UNSPEC entry for host [%s] af: %s.\n", host.get(), + (af == PR_AF_INET) ? "AF_INET" : "AF_INET6")); + + // We need to lock in case any other thread is reading + // addr_info. + MutexAutoLock lock(addrRec->addr_info_lock); + + addrRec->addr_info = nullptr; + addrRec->addr_info_gencnt++; + if (unspecRec->negative) { + rec->negative = unspecRec->negative; + rec->CopyExpirationTimesAndFlagsFrom(unspecRec); + } else if (addrUnspecRec->addr_info) { + // Search for any valid address in the AF_UNSPEC entry + // in the cache (not blacklisted and from the right + // family). + NetAddrElement* addrIter = + addrUnspecRec->addr_info->mAddresses.getFirst(); + while (addrIter) { + if ((af == addrIter->mAddress.inet.family) && + !addrUnspecRec->Blacklisted(&addrIter->mAddress)) { + if (!addrRec->addr_info) { + addrRec->addr_info = + new AddrInfo(addrUnspecRec->addr_info->mHostName, + addrUnspecRec->addr_info->mCanonicalName, + addrUnspecRec->addr_info->IsTRR()); + addrRec->addr_info_gencnt++; + rec->CopyExpirationTimesAndFlagsFrom(unspecRec); + } + addrRec->addr_info->AddAddress(new NetAddrElement(*addrIter)); + } + addrIter = addrIter->getNext(); + } + } + // Now check if we have a new record. + if (rec->HasUsableResult(now, flags)) { + result = rec; + if (rec->negative) { + status = NS_ERROR_UNKNOWN_HOST; + } + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); + ConditionallyRefreshRecord(rec, host); + } else if (af == PR_AF_INET6) { + // For AF_INET6, a new lookup means another AF_UNSPEC + // lookup. We have already iterated through the + // AF_UNSPEC addresses, so we mark this record as + // negative. + LOG( + (" No AF_INET6 in AF_UNSPEC entry: " + "host [%s] unknown host.", + host.get())); + result = rec; + rec->negative = true; + status = NS_ERROR_UNKNOWN_HOST; Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_NEGATIVE_HIT); } - status = NS_ERROR_UNKNOWN_HOST; + } + } + + // If this is a by-type request or if no valid record was found + // in the cache or this is an AF_UNSPEC request, then start a + // new lookup. + if (!result) { + LOG((" No usable record in cache for host [%s] type %d.", host.get(), + type)); + + if (flags & RES_REFRESH_CACHE) { + rec->Invalidate(); } - // Check whether host is a IP address for A/AAAA queries. - // For by-type records we have already checked at the beginning of - // this function. - } else if (addrRec && addrRec->addr) { - // if the host name is an IP address literal and has been - // parsed, go ahead and use it. - LOG((" Using cached address for IP Literal [%s].\n", host.get())); - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); - result = rec; - } else if (addrRec && - PR_StringToNetAddr(host.get(), &tempAddr) == PR_SUCCESS) { - // try parsing the host name as an IP address literal to short - // circuit full host resolution. (this is necessary on some - // platforms like Win9x. see bug 219376 for more details.) - LOG((" Host is IP Literal [%s].\n", host.get())); - - // ok, just copy the result into the host record, and be - // done with it! ;-) - addrRec->addr = MakeUnique(); - PRNetAddrToNetAddr(&tempAddr, addrRec->addr.get()); - // put reference to host record on stack... - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_LITERAL); - result = rec; - - // Check if we have received too many requests. - } else if (mPendingCount >= MAX_NON_PRIORITY_REQUESTS && - !IsHighPriority(flags) && !rec->mResolving) { - LOG( - (" Lookup queue full: dropping %s priority request for " - "host [%s].\n", - IsMediumPriority(flags) ? "medium" : "low", host.get())); - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_OVERFLOW); - } - // This is a lower priority request and we are swamped, so refuse it. - rv = NS_ERROR_DNS_LOOKUP_QUEUE_FULL; - - // Check if the offline flag is set. - } else if (flags & RES_OFFLINE) { - LOG((" Offline request for host [%s]; ignoring.\n", host.get())); - rv = NS_ERROR_OFFLINE; - - // We do not have a valid result till here. - // A/AAAA request can check for an alternative entry like AF_UNSPEC. - // Otherwise we need to start a new query. - } else if (!rec->mResolving) { - // If this is an IPV4 or IPV6 specific request, check if there is - // an AF_UNSPEC entry we can use. Otherwise, hit the resolver... - if (addrRec && !(flags & RES_BYPASS_CACHE) && - ((af == PR_AF_INET) || (af == PR_AF_INET6))) { - // Check for an AF_UNSPEC entry. - - const nsHostKey unspecKey( - host, aTrrServer, nsIDNSService::RESOLVE_TYPE_DEFAULT, flags, - PR_AF_UNSPEC, (aOriginAttributes.mPrivateBrowsingId > 0), - originSuffix); - RefPtr unspecRec = mRecordDB.Get(unspecKey); - - TimeStamp now = TimeStamp::NowLoRes(); - if (unspecRec && unspecRec->HasUsableResult(now, flags)) { - MOZ_ASSERT(unspecRec->IsAddrRecord()); - - RefPtr addrUnspecRec = do_QueryObject(unspecRec); - MOZ_ASSERT(addrUnspecRec); - MOZ_ASSERT(addrUnspecRec->addr_info || addrUnspecRec->negative, - "Entry should be resolved or negative."); - - LOG((" Trying AF_UNSPEC entry for host [%s] af: %s.\n", host.get(), - (af == PR_AF_INET) ? "AF_INET" : "AF_INET6")); - - // We need to lock in case any other thread is reading - // addr_info. - MutexAutoLock lock(addrRec->addr_info_lock); - - addrRec->addr_info = nullptr; - addrRec->addr_info_gencnt++; - if (unspecRec->negative) { - rec->negative = unspecRec->negative; - rec->CopyExpirationTimesAndFlagsFrom(unspecRec); - } else if (addrUnspecRec->addr_info) { - // Search for any valid address in the AF_UNSPEC entry - // in the cache (not blacklisted and from the right - // family). - NetAddrElement* addrIter = - addrUnspecRec->addr_info->mAddresses.getFirst(); - while (addrIter) { - if ((af == addrIter->mAddress.inet.family) && - !addrUnspecRec->Blacklisted(&addrIter->mAddress)) { - if (!addrRec->addr_info) { - addrRec->addr_info = - new AddrInfo(addrUnspecRec->addr_info->mHostName, - addrUnspecRec->addr_info->mCanonicalName, - addrUnspecRec->addr_info->IsTRR()); - addrRec->addr_info_gencnt++; - rec->CopyExpirationTimesAndFlagsFrom(unspecRec); - } - addrRec->addr_info->AddAddress(new NetAddrElement(*addrIter)); - } - addrIter = addrIter->getNext(); - } - } - // Now check if we have a new record. - if (rec->HasUsableResult(now, flags)) { - result = rec; - if (rec->negative) { - status = NS_ERROR_UNKNOWN_HOST; - } - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - ConditionallyRefreshRecord(rec, host); - } else if (af == PR_AF_INET6) { - // For AF_INET6, a new lookup means another AF_UNSPEC - // lookup. We have already iterated through the - // AF_UNSPEC addresses, so we mark this record as - // negative. - LOG( - (" No AF_INET6 in AF_UNSPEC entry: " - "host [%s] unknown host.", - host.get())); - result = rec; - rec->negative = true; - status = NS_ERROR_UNKNOWN_HOST; - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NEGATIVE_HIT); - } - } - } - - // If this is a by-type request or if no valid record was found - // in the cache or this is an AF_UNSPEC request, then start a - // new lookup. - if (!result) { - LOG((" No usable record in cache for host [%s] type %d.", host.get(), - type)); - - if (flags & RES_REFRESH_CACHE) { - rec->Invalidate(); - } - - // Add callback to the list of pending callbacks. - rec->mCallbacks.insertBack(callback); - rec->flags = flags; - rv = NameLookup(rec); - if (IS_ADDR_TYPE(type)) { - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NETWORK_FIRST); - } - if (NS_FAILED(rv) && callback->isInList()) { - callback->remove(); - } else { - LOG( - (" DNS lookup for host [%s] blocking " - "pending 'getaddrinfo' or trr query: " - "callback [%p]", - host.get(), callback.get())); - } - } - - } else if (addrRec && addrRec->mDidCallbacks) { - // This is only for A/AAAA query. - // record is still pending more (TRR) data; make the callback - // at once - result = rec; - // make it count as a hit - Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - - LOG((" Host [%s] re-using early TRR resolve data\n", host.get())); - } else { - LOG( - (" Host [%s] is being resolved. Appending callback " - "[%p].", - host.get(), callback.get())); - + // Add callback to the list of pending callbacks. rec->mCallbacks.insertBack(callback); - - // Only A/AAAA records are place in a queue. The queues are for - // the native resolver, therefore by-type request are never put - // into a queue. - if (addrRec && addrRec->onQueue) { + rec->flags = flags; + rv = NameLookup(rec); + if (IS_ADDR_TYPE(type)) { Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, - METHOD_NETWORK_SHARED); + METHOD_NETWORK_FIRST); + } + if (NS_FAILED(rv) && callback->isInList()) { + callback->remove(); + } else { + LOG( + (" DNS lookup for host [%s] blocking " + "pending 'getaddrinfo' or trr query: " + "callback [%p]", + host.get(), callback.get())); + } + } - // Consider the case where we are on a pending queue of - // lower priority than the request is being made at. - // In that case we should upgrade to the higher queue. + } else if (addrRec && addrRec->mDidCallbacks) { + // This is only for A/AAAA query. + // record is still pending more (TRR) data; make the callback + // at once + result = rec; + // make it count as a hit + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, METHOD_HIT); - if (IsHighPriority(flags) && !IsHighPriority(rec->flags)) { - // Move from (low|med) to high. - NS_ASSERTION(addrRec->onQueue, - "Moving Host Record Not Currently Queued"); - rec->remove(); - mHighQ.insertBack(rec); - rec->flags = flags; - ConditionallyCreateThread(rec); - } else if (IsMediumPriority(flags) && IsLowPriority(rec->flags)) { - // Move from low to med. - NS_ASSERTION(addrRec->onQueue, - "Moving Host Record Not Currently Queued"); - rec->remove(); - mMediumQ.insertBack(rec); - rec->flags = flags; - mIdleTaskCV.Notify(); - } + LOG((" Host [%s] re-using early TRR resolve data\n", host.get())); + } else { + LOG( + (" Host [%s] is being resolved. Appending callback " + "[%p].", + host.get(), callback.get())); + + rec->mCallbacks.insertBack(callback); + + // Only A/AAAA records are place in a queue. The queues are for + // the native resolver, therefore by-type request are never put + // into a queue. + if (addrRec && addrRec->onQueue) { + Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2, + METHOD_NETWORK_SHARED); + + // Consider the case where we are on a pending queue of + // lower priority than the request is being made at. + // In that case we should upgrade to the higher queue. + + if (IsHighPriority(flags) && !IsHighPriority(rec->flags)) { + // Move from (low|med) to high. + NS_ASSERTION(addrRec->onQueue, + "Moving Host Record Not Currently Queued"); + rec->remove(); + mHighQ.insertBack(rec); + rec->flags = flags; + ConditionallyCreateThread(rec); + } else if (IsMediumPriority(flags) && IsLowPriority(rec->flags)) { + // Move from low to med. + NS_ASSERTION(addrRec->onQueue, + "Moving Host Record Not Currently Queued"); + rec->remove(); + mMediumQ.insertBack(rec); + rec->flags = flags; + mIdleTaskCV.Notify(); } } } diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h index b5b549c37e82..440cff7c55b0 100644 --- a/netwerk/dns/nsHostResolver.h +++ b/netwerk/dns/nsHostResolver.h @@ -440,6 +440,14 @@ class nsHostResolver : public nsISupports, public AHostResolver { uint16_t flags, uint16_t af, nsResolveHostCallback* callback); + nsHostRecord* InitRecord(const nsHostKey& key); + + /** + * return a resolved hard coded loopback dns record for the specified key + */ + already_AddRefed InitLoopbackRecord(const nsHostKey& key, + nsresult* aRv); + /** * removes the specified callback from the nsHostRecord for the given * hostname, originAttributes, flags, and address family. these parameters diff --git a/netwerk/test/unit/test_about_networking.js b/netwerk/test/unit/test_about_networking.js index ed015bfc18d8..c423cd23d45f 100644 --- a/netwerk/test/unit/test_about_networking.js +++ b/netwerk/test/unit/test_about_networking.js @@ -89,6 +89,9 @@ function run_test() { true ); + // We always resolve localhost as it's hardcoded without the following pref: + Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); + let ioService = Cc["@mozilla.org/network/io-service;1"].getService( Ci.nsIIOService ); @@ -103,6 +106,7 @@ function run_test() { channel.open(); gServerSocket.init(-1, true, -1); + Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); run_next_test(); } diff --git a/netwerk/test/unit/test_dns_offline.js b/netwerk/test/unit/test_dns_offline.js index 15abfa193932..a82f08040767 100644 --- a/netwerk/test/unit/test_dns_offline.js +++ b/netwerk/test/unit/test_dns_offline.js @@ -43,6 +43,8 @@ const defaultOriginAttributes = {}; function run_test() { do_test_pending(); prefs.setBoolPref("network.dns.offline-localhost", false); + // We always resolve localhost as it's hardcoded without the following pref: + prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); ioService.offline = true; try { dns.asyncResolve( @@ -97,4 +99,5 @@ function test3Continued() { function cleanup() { prefs.clearUserPref("network.dns.offline-localhost"); + prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); } diff --git a/netwerk/test/unit/test_dns_originAttributes.js b/netwerk/test/unit/test_dns_originAttributes.js index b87b6b44fecb..a2857bc7402c 100644 --- a/netwerk/test/unit/test_dns_originAttributes.js +++ b/netwerk/test/unit/test_dns_originAttributes.js @@ -2,6 +2,9 @@ var dns = Cc["@mozilla.org/network/dns-service;1"].getService(Ci.nsIDNSService); var threadManager = Cc["@mozilla.org/thread-manager;1"].getService( Ci.nsIThreadManager ); +var prefs = Cc["@mozilla.org/preferences-service;1"].getService( + Ci.nsIPrefBranch +); var mainThread = threadManager.currentThread; var listener1 = { @@ -64,6 +67,7 @@ function test2() { // for this originAttributes. function test3() { do_test_pending(); + prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); try { dns.asyncResolve( "localhost", @@ -74,6 +78,7 @@ function test3() { ); } catch (e) { Assert.equal(e.result, Cr.NS_ERROR_OFFLINE); + prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); do_test_finished(); } } diff --git a/netwerk/test/unit/test_ping_aboutnetworking.js b/netwerk/test/unit/test_ping_aboutnetworking.js index c72900716a7b..5808a1fbd419 100644 --- a/netwerk/test/unit/test_ping_aboutnetworking.js +++ b/netwerk/test/unit/test_ping_aboutnetworking.js @@ -49,6 +49,8 @@ function run_test() { // disable network changed events to avoid the the risk of having the dns // cache getting flushed behind our back ps.setBoolPref("network.notify.changed", false); + // Localhost is hardcoded to loopback and isn't cached, disable that with this pref + ps.setBoolPref("network.proxy.allow_hijacking_localhost", true); registerCleanupFunction(function() { ps.clearUserPref("network.notify.changed"); diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index 3e5d96296853..e2093abb480a 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -1010,6 +1010,7 @@ add_task(async function test24k() { // resolver if it's not in the excluded domains add_task(async function test25() { dns.clearCache(true); + Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); // Disable localhost hardcoding Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only Services.prefs.setCharPref("network.trr.excluded-domains", ""); Services.prefs.setCharPref("network.trr.builtin-excluded-domains", ""); @@ -1019,6 +1020,7 @@ add_task(async function test25() { ); await new DNSListener("localhost", "192.192.192.192", true); + Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); }); // TRR-only check that localhost goes directly to native lookup when in the excluded-domains From 04828e3a4a83d9bfbe51208c3ac49fa43ea13167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 19 Mar 2020 13:18:16 +0000 Subject: [PATCH 03/27] Bug 253870 - Make disabled form controls selectable. r=masayuki,MarcoZ This rejiggers a bit the way selection focus is handled so that focusing a disabled form control with the mouse handles selection properly, and hides the document selection and so on. This matches the behavior of other browsers as far as I can tell. Given now readonly and disabled editors behave the same, we can simplify a bit the surrounding editor code. Differential Revision: https://phabricator.services.mozilla.com/D66464 --HG-- extra : moz-landing-system : lando --- .../mochitest/textselection/test_general.html | 6 ++ dom/base/Document.cpp | 23 +++++ dom/base/Document.h | 1 + dom/base/DocumentOrShadowRoot.cpp | 24 ++--- dom/base/nsCopySupport.cpp | 30 ++---- dom/base/nsCopySupport.h | 4 +- dom/base/nsISelectionController.idl | 17 ++++ dom/base/test/mochitest.ini | 1 + dom/base/test/test_copypaste_disabled.html | 90 +++++++++++++++++ dom/events/ContentEventHandler.cpp | 12 +-- dom/events/EventStateManager.cpp | 5 +- dom/html/HTMLInputElement.cpp | 15 +-- dom/html/TextControlState.cpp | 56 ++++++----- dom/html/TextControlState.h | 4 - editor/libeditor/EditorBase.cpp | 97 ++---------------- editor/libeditor/EditorBase.h | 7 +- editor/libeditor/EditorEventListener.cpp | 16 ++- editor/libeditor/HTMLEditUtils.h | 3 + editor/libeditor/HTMLEditor.cpp | 6 +- editor/libeditor/HTMLEditorDataTransfer.cpp | 12 +-- editor/libeditor/TextEditSubActionHandler.cpp | 14 +-- editor/libeditor/TextEditor.cpp | 8 +- editor/nsIEditor.idl | 22 ++--- editor/reftests/reftest.list | 6 +- .../selection_visibility_after_reframe-2.html | 1 + .../selection_visibility_after_reframe-3.html | 1 + layout/base/PresShell.cpp | 98 +++++++++++++------ layout/base/PresShell.h | 30 +++--- layout/base/nsDocumentViewer.cpp | 2 +- layout/forms/nsTextControlFrame.cpp | 41 ++------ layout/generic/nsFrameSelection.cpp | 2 + widget/tests/test_imestate.html | 4 +- 32 files changed, 347 insertions(+), 311 deletions(-) create mode 100644 dom/base/test/test_copypaste_disabled.html diff --git a/accessible/tests/mochitest/textselection/test_general.html b/accessible/tests/mochitest/textselection/test_general.html index 5ccc225048d6..a8b9137b1aae 100644 --- a/accessible/tests/mochitest/textselection/test_general.html +++ b/accessible/tests/mochitest/textselection/test_general.html @@ -126,6 +126,12 @@ ]; this.invoke = function changeDOMSelection_invoke() { + // HyperTextAccessible::GetSelectionDOMRanges ignores hidden selections. + // Here we may be focusing an editable element (and thus hiding the + // main document selection), so blur it so that we test what we want to + // test. + document.activeElement.blur(); + var sel = window.getSelection(); var range = document.createRange(); range.setStart(getNode(aNodeID1), aNodeOffset1); diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index f982b055ab49..10c1d35d722f 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -5933,6 +5933,29 @@ Nullable Document::GetDefaultView() const { return WindowProxyHolder(win->GetBrowsingContext()); } +nsIContent* Document::GetUnretargetedFocusedContent() const { + nsCOMPtr window = GetWindow(); + if (!window) { + return nullptr; + } + nsCOMPtr focusedWindow; + nsIContent* focusedContent = nsFocusManager::GetFocusedDescendant( + window, nsFocusManager::eOnlyCurrentWindow, + getter_AddRefs(focusedWindow)); + if (!focusedContent) { + return nullptr; + } + // be safe and make sure the element is from this document + if (focusedContent->OwnerDoc() != this) { + return nullptr; + } + + if (focusedContent->ChromeOnlyAccess()) { + return focusedContent->FindFirstNonChromeOnlyAccessContent(); + } + return focusedContent; +} + Element* Document::GetActiveElement() { // Get the focused element. Element* focusedElement = GetRetargetedFocusedElement(); diff --git a/dom/base/Document.h b/dom/base/Document.h index 41603bb8acfa..8e6f3a2c2e77 100644 --- a/dom/base/Document.h +++ b/dom/base/Document.h @@ -3293,6 +3293,7 @@ class Document : public nsINode, mozilla::ErrorResult& rv); Nullable GetDefaultView() const; Element* GetActiveElement(); + nsIContent* GetUnretargetedFocusedContent() const; bool HasFocus(ErrorResult& rv) const; void GetDesignMode(nsAString& aDesignMode); void SetDesignMode(const nsAString& aDesignMode, diff --git a/dom/base/DocumentOrShadowRoot.cpp b/dom/base/DocumentOrShadowRoot.cpp index 659004e0c0be..a15efda70ed3 100644 --- a/dom/base/DocumentOrShadowRoot.cpp +++ b/dom/base/DocumentOrShadowRoot.cpp @@ -256,25 +256,13 @@ nsIContent* DocumentOrShadowRoot::Retarget(nsIContent* aContent) const { } Element* DocumentOrShadowRoot::GetRetargetedFocusedElement() { - if (nsCOMPtr window = AsNode().OwnerDoc()->GetWindow()) { - nsCOMPtr focusedWindow; - nsIContent* focusedContent = nsFocusManager::GetFocusedDescendant( - window, nsFocusManager::eOnlyCurrentWindow, - getter_AddRefs(focusedWindow)); - // be safe and make sure the element is from this document - if (focusedContent && focusedContent->OwnerDoc() == AsNode().OwnerDoc()) { - if (focusedContent->ChromeOnlyAccess()) { - focusedContent = focusedContent->FindFirstNonChromeOnlyAccessContent(); - } - - if (focusedContent) { - if (nsIContent* retarget = Retarget(focusedContent)) { - return retarget->AsElement(); - } - } - } + auto* content = AsNode().OwnerDoc()->GetUnretargetedFocusedContent(); + if (!content) { + return nullptr; + } + if (nsIContent* retarget = Retarget(content)) { + return retarget->AsElement(); } - return nullptr; } diff --git a/dom/base/nsCopySupport.cpp b/dom/base/nsCopySupport.cpp index b6f57eff3d72..de68d30810b0 100644 --- a/dom/base/nsCopySupport.cpp +++ b/dom/base/nsCopySupport.cpp @@ -17,6 +17,7 @@ #include "imgIContainer.h" #include "imgIRequest.h" #include "nsFocusManager.h" +#include "nsFrameSelection.h" #include "mozilla/dom/DataTransfer.h" #include "nsIDocShell.h" @@ -673,37 +674,27 @@ static nsresult AppendImagePromise(nsITransferable* aTransferable, } #endif // XP_WIN -nsIContent* nsCopySupport::GetSelectionForCopy(Document* aDocument, - Selection** aSelection) { - *aSelection = nullptr; - +already_AddRefed nsCopySupport::GetSelectionForCopy( + Document* aDocument) { PresShell* presShell = aDocument->GetPresShell(); if (!presShell) { return nullptr; } - nsCOMPtr focusedContent; - nsCOMPtr selectionController = - presShell->GetSelectionControllerForFocusedContent( - getter_AddRefs(focusedContent)); - if (!selectionController) { + RefPtr frameSel = presShell->GetLastFocusedFrameSelection(); + if (!frameSel) { return nullptr; } - RefPtr sel = selectionController->GetSelection( - nsISelectionController::SELECTION_NORMAL); - sel.forget(aSelection); - return focusedContent; + RefPtr sel = frameSel->GetSelection(SelectionType::eNormal); + return sel.forget(); } bool nsCopySupport::CanCopy(Document* aDocument) { if (!aDocument) return false; - RefPtr sel; - GetSelectionForCopy(aDocument, getter_AddRefs(sel)); - NS_ENSURE_TRUE(sel, false); - - return !sel->IsCollapsed(); + RefPtr sel = GetSelectionForCopy(aDocument); + return sel && !sel->IsCollapsed(); } static bool IsInsideRuby(nsINode* aNode) { @@ -717,7 +708,6 @@ static bool IsInsideRuby(nsINode* aNode) { static bool IsSelectionInsideRuby(Selection* aSelection) { uint32_t rangeCount = aSelection->RangeCount(); - ; for (auto i : IntegerRange(rangeCount)) { nsRange* range = aSelection->GetRangeAt(i); if (!IsInsideRuby(range->GetClosestCommonInclusiveAncestor())) { @@ -776,7 +766,7 @@ bool nsCopySupport::FireClipboardEvent(EventMessage aEventMessage, // If a selection was not supplied, try to find it. RefPtr sel = aSelection; if (!sel) { - GetSelectionForCopy(doc, getter_AddRefs(sel)); + sel = GetSelectionForCopy(doc); } // Retrieve the event target node from the start of the selection. diff --git a/dom/base/nsCopySupport.h b/dom/base/nsCopySupport.h index 1d6141a7b848..c7805363ab9e 100644 --- a/dom/base/nsCopySupport.h +++ b/dom/base/nsCopySupport.h @@ -69,8 +69,8 @@ class nsCopySupport { * and this focused content node returned. Otherwise, aSelection will be * set to the document's selection and null will be returned. */ - static nsIContent* GetSelectionForCopy(mozilla::dom::Document* aDocument, - mozilla::dom::Selection** aSelection); + static already_AddRefed GetSelectionForCopy( + mozilla::dom::Document* aDocument); /** * Returns true if a copy operation is currently permitted based on the diff --git a/dom/base/nsISelectionController.idl b/dom/base/nsISelectionController.idl index 0a4b6057abf8..bc7dce6ce9ec 100644 --- a/dom/base/nsISelectionController.idl +++ b/dom/base/nsISelectionController.idl @@ -83,6 +83,23 @@ interface nsISelectionController : nsISelectionDisplay [noscript,nostdcall,notxpcom,binaryname(GetSelection)] Selection getDOMSelection(in short aType); + /** + * Called when the selection controller should take the focus. + * + * This will take care to hide the previously-focused selection, show this + * selection, and repaint both. + */ + [noscript,nostdcall,notxpcom] + void selectionWillTakeFocus(); + + /** + * Called when the selection controller has lost the focus. + * + * This will take care to hide and repaint the selection. + */ + [noscript,nostdcall,notxpcom] + void selectionWillLoseFocus(); + const short SCROLL_SYNCHRONOUS = 1<<1; const short SCROLL_FIRST_ANCESTOR_ONLY = 1<<2; const short SCROLL_CENTER_VERTICALLY = 1<<4; diff --git a/dom/base/test/mochitest.ini b/dom/base/test/mochitest.ini index 4d71bbadbf68..9910f5f38ba0 100644 --- a/dom/base/test/mochitest.ini +++ b/dom/base/test/mochitest.ini @@ -635,6 +635,7 @@ skip-if = toolkit == 'android' || headless #bug 904183 skip-if = toolkit == 'android' || headless #bug 904183 [test_copypaste.xhtml] skip-if = headless #bug 904183 +[test_copypaste_disabled.html] [test_createHTMLDocument.html] [test_data_uri.html] skip-if = verify diff --git a/dom/base/test/test_copypaste_disabled.html b/dom/base/test/test_copypaste_disabled.html new file mode 100644 index 000000000000..a1a0ab3c2d7b --- /dev/null +++ b/dom/base/test/test_copypaste_disabled.html @@ -0,0 +1,90 @@ + + + + + + + + efgh
mnop + + diff --git a/dom/events/ContentEventHandler.cpp b/dom/events/ContentEventHandler.cpp index 4ff263f8ef4b..593ecb7a736f 100644 --- a/dom/events/ContentEventHandler.cpp +++ b/dom/events/ContentEventHandler.cpp @@ -317,16 +317,15 @@ nsresult ContentEventHandler::InitCommon(SelectionType aSelectionType, nsresult rv = InitBasic(aRequireFlush); NS_ENSURE_SUCCESS(rv, rv); - nsCOMPtr selectionController; + RefPtr frameSel; if (PresShell* presShell = mDocument->GetPresShell()) { - selectionController = presShell->GetSelectionControllerForFocusedContent(); + frameSel = presShell->GetLastFocusedFrameSelection(); } - if (NS_WARN_IF(!selectionController)) { + if (NS_WARN_IF(!frameSel)) { return NS_ERROR_NOT_AVAILABLE; } - mSelection = - selectionController->GetSelection(ToRawSelectionType(aSelectionType)); + mSelection = frameSel->GetSelection(aSelectionType); if (NS_WARN_IF(!mSelection)) { return NS_ERROR_NOT_AVAILABLE; } @@ -335,8 +334,7 @@ nsresult ContentEventHandler::InitCommon(SelectionType aSelectionType, if (mSelection->Type() == SelectionType::eNormal) { normalSelection = mSelection; } else { - normalSelection = selectionController->GetSelection( - nsISelectionController::SELECTION_NORMAL); + normalSelection = frameSel->GetSelection(SelectionType::eNormal); if (NS_WARN_IF(!normalSelection)) { return NS_ERROR_NOT_AVAILABLE; } diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index c673b99a3602..ec476d580233 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -5165,7 +5165,7 @@ nsresult EventStateManager::HandleMiddleClickPaste( if (NS_WARN_IF(!document)) { return NS_ERROR_FAILURE; } - nsCopySupport::GetSelectionForCopy(document, getter_AddRefs(selection)); + selection = nsCopySupport::GetSelectionForCopy(document); if (NS_WARN_IF(!selection)) { return NS_ERROR_FAILURE; } @@ -5214,8 +5214,7 @@ nsresult EventStateManager::HandleMiddleClickPaste( } // Check if the editor is still the good target to paste. - if (aTextEditor->Destroyed() || aTextEditor->IsReadonly() || - aTextEditor->IsDisabled()) { + if (aTextEditor->Destroyed() || aTextEditor->IsReadonly()) { // XXX Should we consume the event when the editor is readonly and/or // disabled? return NS_OK; diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 93c75f18760e..f2259b774134 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -6639,18 +6639,13 @@ void HTMLInputElement::OnValueChanged(ValueChangeKind aKind) { } bool HTMLInputElement::HasCachedSelection() { - bool isCached = false; TextControlState* state = GetEditorState(); - if (state) { - isCached = state->IsSelectionCached() && - state->HasNeverInitializedBefore() && - state->GetSelectionProperties().GetStart() != - state->GetSelectionProperties().GetEnd(); - if (isCached) { - state->WillInitEagerly(); - } + if (!state) { + return false; } - return isCached; + return state->IsSelectionCached() && state->HasNeverInitializedBefore() && + state->GetSelectionProperties().GetStart() != + state->GetSelectionProperties().GetEnd(); } void HTMLInputElement::FieldSetDisabledChanged(bool aNotify) { diff --git a/dom/html/TextControlState.cpp b/dom/html/TextControlState.cpp index 5a8328de449f..1814e3945dfe 100644 --- a/dom/html/TextControlState.cpp +++ b/dom/html/TextControlState.cpp @@ -174,10 +174,6 @@ class RestoreSelectionState : public Runnable { mFrame->SetSelectionRange(properties.GetStart(), properties.GetEnd(), properties.GetDirection()); } - if (!mTextControlState->mSelectionRestoreEagerInit) { - mTextControlState->HideSelectionIfBlurred(); - } - mTextControlState->mSelectionRestoreEagerInit = false; } if (mTextControlState) { @@ -217,7 +213,6 @@ class MOZ_RAII AutoRestoreEditorState final { // appearing the method in profile. So, this class should check if it's // necessary to call. uint32_t flags = mSavedFlags; - flags &= ~(nsIEditor::eEditorDisabledMask); flags &= ~(nsIEditor::eEditorReadonlyMask); flags |= nsIEditor::eEditorDontEchoPassword; if (mSavedFlags != flags) { @@ -361,6 +356,8 @@ class TextInputSelectionController final : public nsSupportsWeakReference, int16_t aStartOffset, int16_t aEndOffset, bool* aRetval) override; + void SelectionWillTakeFocus() override; + void SelectionWillLoseFocus() override; private: RefPtr mFrameSelection; @@ -771,6 +768,22 @@ TextInputSelectionController::SelectAll() { return frameSelection->SelectAll(); } +void TextInputSelectionController::SelectionWillTakeFocus() { + if (mFrameSelection) { + if (PresShell* shell = mFrameSelection->GetPresShell()) { + shell->FrameSelectionWillTakeFocus(*mFrameSelection); + } + } +} + +void TextInputSelectionController::SelectionWillLoseFocus() { + if (mFrameSelection) { + if (PresShell* shell = mFrameSelection->GetPresShell()) { + shell->FrameSelectionWillLoseFocus(*mFrameSelection); + } + } +} + NS_IMETHODIMP TextInputSelectionController::CheckVisibility(nsINode* node, int16_t startOffset, @@ -1396,7 +1409,6 @@ TextControlState::TextControlState(TextControlElement* aOwningElement) mEditorInitialized(false), mValueTransferInProgress(false), mSelectionCached(true), - mSelectionRestoreEagerInit(false), mPlaceholderVisibility(false), mPreviewVisibility(false) // When adding more member variable initializations here, add the same @@ -1419,7 +1431,6 @@ TextControlState* TextControlState::Construct( state->mEditorInitialized = false; state->mValueTransferInProgress = false; state->mSelectionCached = true; - state->mSelectionRestoreEagerInit = false; state->mPlaceholderVisibility = false; state->mPreviewVisibility = false; // When adding more member variable initializations here, add the same @@ -1640,7 +1651,9 @@ nsresult TextControlState::BindToFrame(nsTextControlFrame* aFrame) { mTextListener = new TextInputListener(mTextCtrlElement); mTextListener->SetFrame(mBoundFrame); - mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); + + // Editor will override this as needed from InitializeSelection. + mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_HIDDEN); // Get the caret and make it a selection listener. // FYI: It's safe to use raw pointer for calling @@ -1884,21 +1897,13 @@ nsresult TextControlState::PrepareEditor(const nsAString* aValue) { editorFlags = newTextEditor->Flags(); // Check if the readonly attribute is set. - if (mTextCtrlElement->HasAttr(kNameSpaceID_None, nsGkAtoms::readonly)) { + // + // TODO: Should probably call IsDisabled(), as it is cheaper. + if (mTextCtrlElement->HasAttr(kNameSpaceID_None, nsGkAtoms::readonly) || + mTextCtrlElement->HasAttr(kNameSpaceID_None, nsGkAtoms::disabled)) { editorFlags |= nsIEditor::eEditorReadonlyMask; } - // Check if the disabled attribute is set. - // TODO: call IsDisabled() here! - if (mTextCtrlElement->HasAttr(kNameSpaceID_None, nsGkAtoms::disabled)) { - editorFlags |= nsIEditor::eEditorDisabledMask; - } - - // Disable the selection if necessary. - if (newTextEditor->IsDisabled()) { - mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_OFF); - } - SetEditorFlagsIfNecessary(*newTextEditor, editorFlags); if (shouldInitializeEditor) { @@ -2385,6 +2390,10 @@ void TextControlState::UnbindFromFrame(nsTextControlFrame* aFrame) { AutoTextControlHandlingState handlingUnbindFromFrame( *this, TextControlAction::UnbindFromFrame); + if (mSelCon) { + mSelCon->SelectionWillLoseFocus(); + } + // We need to start storing the value outside of the editor if we're not // going to use it anymore, so retrieve it for now. nsAutoString value; @@ -3087,13 +3096,6 @@ void TextControlState::UpdateOverlayTextVisibility(bool aNotify) { } } -void TextControlState::HideSelectionIfBlurred() { - MOZ_ASSERT(mSelCon, "Should have a selection controller if we have a frame!"); - if (!nsContentUtils::IsFocusedContent(mTextCtrlElement)) { - mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_HIDDEN); - } -} - bool TextControlState::EditorHasComposition() { return mTextEditor && mTextEditor->IsIMEComposing(); } diff --git a/dom/html/TextControlState.h b/dom/html/TextControlState.h index 5bc9d886d397..9f0b3c879f34 100644 --- a/dom/html/TextControlState.h +++ b/dom/html/TextControlState.h @@ -274,7 +274,6 @@ class TextControlState final : public SupportsWeakPtr { void SetPreviewText(const nsAString& aValue, bool aNotify); void GetPreviewText(nsAString& aValue); bool GetPreviewVisibility() { return mPreviewVisibility; } - void HideSelectionIfBlurred(); struct SelectionProperties { public: @@ -315,7 +314,6 @@ class TextControlState final : public SupportsWeakPtr { bool IsSelectionCached() const { return mSelectionCached; } SelectionProperties& GetSelectionProperties() { return mSelectionProperties; } MOZ_CAN_RUN_SCRIPT void SetSelectionProperties(SelectionProperties& aProps); - void WillInitEagerly() { mSelectionRestoreEagerInit = true; } bool HasNeverInitializedBefore() const { return !mEverInited; } // Sync up our selection properties with our editor prior to being destroyed. // This will invoke UnbindFromFrame() to ensure that we grab whatever @@ -456,8 +454,6 @@ class TextControlState final : public SupportsWeakPtr { bool mValueTransferInProgress; // Whether a value is being transferred to the // frame bool mSelectionCached; // Whether mSelectionProperties is valid - mutable bool mSelectionRestoreEagerInit; // Whether we're eager initing - // because of selection restore bool mPlaceholderVisibility; bool mPreviewVisibility; diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 82d4dd91c8e6..31c22a24f204 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -285,12 +285,6 @@ nsresult EditorBase::Init(Document& aDocument, Element* aRoot, NS_WARNING_ASSERTION( NS_SUCCEEDED(rvIgnored), "nsISelectionController::SetCaretReadOnly(false) failed, but ignored"); - rvIgnored = selectionController->SetDisplaySelection( - nsISelectionController::SELECTION_ON); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::SetDisplaySelection(nsISelectionController::" - "SELECTION_ON) failed, but ignored"); // Show all the selection reflected to user. rvIgnored = selectionController->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); @@ -309,7 +303,7 @@ nsresult EditorBase::Init(Document& aDocument, Element* aRoot, // Make sure that the editor will be destroyed properly mDidPreDestroy = false; - // Make sure that the ediotr will be created properly + // Make sure that the editor will be created properly mDidPostCreate = false; return NS_OK; @@ -2505,7 +2499,7 @@ nsresult EditorBase::GetPreferredIMEState(IMEState* aState) { aState->mEnabled = IMEState::ENABLED; aState->mOpen = IMEState::DONT_CHANGE_OPEN_STATE; - if (IsReadonly() || IsDisabled()) { + if (IsReadonly()) { aState->mEnabled = IMEState::DISABLED; return NS_OK; } @@ -5039,7 +5033,7 @@ nsresult EditorBase::HandleKeyPressEvent(WidgetKeyboardEvent* aKeyboardEvent) { "HandleKeyPressEvent gets non-keypress event"); // if we are readonly or disabled, then do nothing. - if (IsReadonly() || IsDisabled()) { + if (IsReadonly()) { // consume backspace for disabled and readonly textfields, to prevent // back in history, which could be confusing to users if (aKeyboardEvent->mKeyCode == NS_VK_BACK) { @@ -5134,21 +5128,13 @@ nsresult EditorBase::InitializeSelection(EventTarget* aFocusEventTarget) { caret->SetIgnoreUserModify(targetNode->OwnerDoc()->HasFlag(NODE_IS_EDITABLE)); // Init selection - rvIgnored = selectionController->SetDisplaySelection( - nsISelectionController::SELECTION_ON); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::SetDisplaySelection() failed, but ignored"); rvIgnored = selectionController->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); NS_WARNING_ASSERTION( NS_SUCCEEDED(rvIgnored), "nsISelectionController::SetSelectionFlags() failed, but ignored"); - rvIgnored = selectionController->RepaintSelection( - nsISelectionController::SELECTION_NORMAL); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::RepaintSelection() failed, but ignored"); + + selectionController->SelectionWillTakeFocus(); // If the computed selection root isn't root content, we should set it // as selection ancestor limit. However, if that is root element, it means @@ -5192,26 +5178,6 @@ nsresult EditorBase::InitializeSelection(EventTarget* aFocusEventTarget) { return NS_OK; } -class RepaintSelectionRunner final : public Runnable { - public: - explicit RepaintSelectionRunner(nsISelectionController* aSelectionController) - : Runnable("RepaintSelectionRunner"), - mSelectionController(aSelectionController) {} - - NS_IMETHOD Run() override { - DebugOnly rvIgnored = mSelectionController->RepaintSelection( - nsISelectionController::SELECTION_NORMAL); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::RepaintSelection(nsISelectionController::" - "SELECTION_NORMAL) failed, but ignored"); - return NS_OK; - } - - private: - nsCOMPtr mSelectionController; -}; - nsresult EditorBase::FinalizeSelection() { nsCOMPtr selectionController = GetSelectionController(); @@ -5243,58 +5209,11 @@ nsresult EditorBase::FinalizeSelection() { return NS_ERROR_NOT_INITIALIZED; } focusManager->UpdateCaretForCaretBrowsingMode(); - - if (!HasIndependentSelection()) { - // If this editor doesn't have an independent selection, i.e., it must - // mean that it is an HTML editor, the selection controller is shared with - // presShell. So, even this editor loses focus, other part of the document - // may still have focus. - RefPtr doc = GetDocument(); - ErrorResult ret; - if (!doc || !doc->HasFocus(ret)) { - // If the document already lost focus, mark the selection as disabled. - DebugOnly rvIgnored = selectionController->SetDisplaySelection( - nsISelectionController::SELECTION_DISABLED); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::SetDisplaySelection(nsISelectionController::" - "SELECTION_DISABLED) failed, but ignored"); - } else { - // Otherwise, mark selection as normal because outside of a - // contenteditable element should be selected with normal selection - // color after here. - DebugOnly rvIgnored = selectionController->SetDisplaySelection( - nsISelectionController::SELECTION_ON); - NS_WARNING_ASSERTION( - NS_SUCCEEDED(rvIgnored), - "nsISelectionController::SetDisplaySelection(nsISelectionController::" - "SELECTION_ON) failed, but ignored"); + if (nsCOMPtr node = do_QueryInterface(GetDOMEventTarget())) { + if (node->OwnerDoc()->GetUnretargetedFocusedContent() != node) { + selectionController->SelectionWillLoseFocus(); } - } else if (IsFormWidget() || IsPasswordEditor() || IsReadonly() || - IsDisabled() || IsInputFiltered()) { - // In or