Closed Bug 1472158 Opened 6 years ago Closed 5 years ago

No blobURL broadcasting

Categories

(Core :: DOM: File, enhancement, P2)

Product:
Core
Component:
DOM: File
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla74
Project Flags:
Fission Milestone M6
Tracking Flags:
Tracking Status
firefox74 --- fixed

People

(Reporter: baku, Assigned: baku)

References

(Blocks 2 open bugs)

Details

Attachments

(8 files, 8 obsolete files)

part 1 - Introduce BlobURLProtocolHandler::GetBlobURLPrincipalOrOwnerURI
(deleted), patch
smaug
: review-
Details | Diff | Splinter Review
part 2 - Retrieve the principal from BlobURLChannel
(deleted), patch
smaug
: review-
Details | Diff | Splinter Review
part 3 - stop broadcatsing blobURLs
(deleted), patch
smaug
: review+
Details | Diff | Splinter Review
part 6 - tests
(deleted), patch
smaug
: review-
Details | Diff | Splinter Review
part 7 - documentation
(deleted), patch
smaug
: review-
Details | Diff | Splinter Review
part 4 - Storing BlobURLs in the parent process
(deleted), patch
smaug
: review+
Details | Diff | Splinter Review
part 5 - Retrieve blobURL from the parent process
(deleted), patch
smaug
: review-
Details | Diff | Splinter Review
Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre,asuth
(deleted), text/x-phabricator-request
Details
BlobURLs are currently broadcasted to any process. This is expensive from a memory point of view, and it requires several IPC calls each time a blobURL is created or revoked. This bug is about removing the blobURL broadcasting.
I'm going to ask for a review when the dependency bug is landed.
Attached patch part 4 - BlobURLInputStream for remote BlobURLs (obsolete) (deleted) — — Splinter Review
Priority: -- → P2
Attached patch part 5 - tests (obsolete) (deleted) — — Splinter Review
Blocks: 1438214
Attachment #8988714 - Attachment is obsolete: true
Attachment #9004511 - Flags: review?(bugs)
Attachment #8988715 - Attachment is obsolete: true
Attachment #9004513 - Flags: review?(bugs)
Attachment #8988716 - Attachment is obsolete: true
Attachment #9004514 - Flags: review?(bugs)
Attached patch part 4 - Storing BlobURLs in the parent process (obsolete) (deleted) — — Splinter Review
Attachment #8988717 - Attachment is obsolete: true
Attachment #9004515 - Flags: review?(bugs)
Attachment #8989135 - Attachment is obsolete: true
Attachment #9004516 - Flags: review?(bugs)
Attached patch part 6 - tests (deleted) — — Splinter Review
Attachment #9004518 - Flags: review?(bugs)
Attached patch part 7 - documentation (deleted) — — Splinter Review
Attachment #9004519 - Flags: review?(bugs)
Attachment #9004515 - Attachment is obsolete: true
Attachment #9004515 - Flags: review?(bugs)
Attachment #9004683 - Flags: review?(bugs)
Attachment #9004516 - Attachment is obsolete: true
Attachment #9004516 - Flags: review?(bugs)
Attachment #9004684 - Flags: review?(bugs)
Fully green on try.
Attachment #9004684 - Attachment is obsolete: true
Attachment #9004684 - Flags: review?(bugs)
Attachment #9004690 - Flags: review?(bugs)
Comment on attachment 9004519 [details] [diff] [review] part 7 - documentation >+/** >+ * BlobURLs >+ * ~~~~~~~~ >+ * >+ * A blobURL is a URL connected to a DOM Blob content. It can be created and >+ * used by any process. Each blobURL is registered in a hashtable in each >+ * process, really, each process? I thought the whole point was to avoid that. >+ * BlobURLs and e10s >+ * ~~~~~~~~~~~~~~~~~ >+ * >+ * Another possible scenario is that, the current process wants to open an >+ * unknown blobURL. What is the other scenario? Something above? This isn't quite clear. >+ * When a blobURL needs to be used, if it's unknown, a BlobURLChannel is >+ * created, and this creates a BlobURLInputStream, which sends an IPC message to >+ * the parent asking for that particular blobURL, as string >+ * (ContentChild::RetrieveBlobURL) >+ * >+ * If the parent process known this blobURL, asynchornsly, s/knows/ >+ it sends an IPC s/asynchornsly/asynchronously/ but the sentence is a bit odd. asynchornsly seems to refer to knowning about the bloburl, not about sending it. Please rephrase. >+ * message to the content process, passing the BlobImpl and its nsIPrincipal >+ * (ContentParent::SendBlobURLReady) >+ * >+ * When BlobURLInputStream receives these objects, it reads data from the >+ * BlobImpl's nsIInputStream when needed. In the meantime, BlobURLInputStream >+ * behaves as a nsIAsyncInputStream. 'In the meantime' seems to be useless part of the sentence. Or does BlobURLInputStream behave as nsIAsyncInputStream only for some time? >+ * When a blobURL, marked as revoked, needs to be opened, we return an error. >+ * Otherwise, we continue as described previously. I don't understand this explanation. So, revoked blobURL can't be opened, but what does "otherwise" refer to here?
Attachment #9004519 - Flags: review?(bugs) → review-
Comment on attachment 9004518 [details] [diff] [review] part 6 - tests ok, this contains more than just tests. We need to somehow ensure the blobs are in different processes. And need to have a test for revoking in one process and trying to use in another.
Attachment #9004518 - Flags: review?(bugs) → review-
Attachment #9004514 - Flags: review?(bugs) → review+
Comment on attachment 9004511 [details] [diff] [review] part 1 - Introduce BlobURLProtocolHandler::GetBlobURLPrincipalOrOwnerURI I don't understand the behavior of the new method. It returns blob url's principal or a null principal, but url is always just the blob url's principal's url. and owner URI is returned even if blob url was revoked. Rather odd inconsistency, and changes for example the behavior of NS_SecurityCompareURIs.
Attachment #9004511 - Flags: review?(bugs) → review-
Comment on attachment 9004513 [details] [diff] [review] part 2 - Retrieve the principal from BlobURLChannel ># HG changeset patch ># User Andrea Marchesini <amarchesini@mozilla.com> ># Parent 42eef1146aac56d92671881798f1062dff18b9f7 > >diff --git a/caps/NullPrincipalURI.cpp b/caps/NullPrincipalURI.cpp >--- a/caps/NullPrincipalURI.cpp >+++ b/caps/NullPrincipalURI.cpp >@@ -27,17 +27,16 @@ NullPrincipalURI::NullPrincipalURI() > NullPrincipalURI::NullPrincipalURI(const NullPrincipalURI& aOther) > { > mPath.Assign(aOther.mPath); > } > > nsresult > NullPrincipalURI::Init() > { >- // FIXME: bug 327161 -- make sure the uuid generator is reseeding-resistant. > nsCOMPtr<nsIUUIDGenerator> uuidgen = services::GetUUIDGenerator(); > NS_ENSURE_TRUE(uuidgen, NS_ERROR_NOT_AVAILABLE); > > nsID id; > nsresult rv = uuidgen->GenerateUUIDInPlace(&id); > NS_ENSURE_SUCCESS(rv, rv); > > mPath.SetLength(NSID_LENGTH - 1); // -1 because NSID_LENGTH counts the '\0' >diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp >--- a/caps/nsScriptSecurityManager.cpp >+++ b/caps/nsScriptSecurityManager.cpp >@@ -53,16 +53,17 @@ > #include "nsIURIFixup.h" > #include "nsCDefaultURIFixup.h" > #include "nsIChromeRegistry.h" > #include "nsIResProtocolHandler.h" > #include "nsIContentSecurityPolicy.h" > #include "nsIAsyncVerifyRedirectCallback.h" > #include "mozilla/Preferences.h" > #include "mozilla/dom/BindingUtils.h" >+#include "mozilla/dom/BlobURLChannel.h" > #include "mozilla/NullPrincipal.h" > #include <stdint.h> > #include "mozilla/dom/ScriptSettings.h" > #include "mozilla/ClearOnShutdown.h" > #include "mozilla/StaticPtr.h" > #include "nsContentUtils.h" > #include "nsJSUtils.h" > #include "nsILoadInfo.h" >@@ -341,22 +342,22 @@ nsScriptSecurityManager::GetChannelResul > if (owner) { > CallQueryInterface(owner, aPrincipal); > if (*aPrincipal) { > return NS_OK; > } > } > > if (loadInfo) { >- if (!aIgnoreSandboxing && loadInfo->GetLoadingSandboxed()) { >- MOZ_ALWAYS_TRUE(NS_SUCCEEDED(loadInfo->GetSandboxedLoadingPrincipal(aPrincipal))); >- MOZ_ASSERT(*aPrincipal); >- InheritAndSetCSPOnPrincipalIfNeeded(aChannel, *aPrincipal); >- return NS_OK; >- } >+ if (!aIgnoreSandboxing && loadInfo->GetLoadingSandboxed()) { >+ MOZ_ALWAYS_TRUE(NS_SUCCEEDED(loadInfo->GetSandboxedLoadingPrincipal(aPrincipal))); >+ MOZ_ASSERT(*aPrincipal); >+ InheritAndSetCSPOnPrincipalIfNeeded(aChannel, *aPrincipal); >+ return NS_OK; >+ } > > bool forceInherit = loadInfo->GetForceInheritPrincipal(); > if (aIgnoreSandboxing && !forceInherit) { > // Check if SEC_FORCE_INHERIT_PRINCIPAL was dropped because of > // sandboxing: > if (loadInfo->GetLoadingSandboxed() && > loadInfo->GetForceInheritPrincipalDropped()) { > forceInherit = true; >@@ -389,16 +390,26 @@ nsScriptSecurityManager::GetChannelResul > uri, > inheritForAboutBlank, > false)) { > principalToInherit.forget(aPrincipal); > return NS_OK; > } > } > } >+ >+ nsCOMPtr<nsIBlobURLChannel> blobURLChannel = do_QueryInterface(aChannel); >+ if (blobURLChannel) { >+ nsCOMPtr<nsIPrincipal> principal = blobURLChannel->GetPrincipal(); >+ if (principal) { >+ principal.forget(aPrincipal); >+ return NS_OK; >+ } >+ } >+ > nsresult rv = GetChannelURIPrincipal(aChannel, aPrincipal); > NS_ENSURE_SUCCESS(rv, rv); > InheritAndSetCSPOnPrincipalIfNeeded(aChannel, *aPrincipal); > return NS_OK; > } > > /* The principal of the URI that this channel is loading. This is never > * affected by things like sandboxed loads, or loads where we forcefully >diff --git a/dom/file/uri/BlobURLChannel.cpp b/dom/file/uri/BlobURLChannel.cpp >--- a/dom/file/uri/BlobURLChannel.cpp >+++ b/dom/file/uri/BlobURLChannel.cpp >@@ -4,16 +4,18 @@ > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "BlobURLChannel.h" > #include "mozilla/dom/BlobImpl.h" > > using namespace mozilla::dom; > >+NS_IMPL_ISUPPORTS_INHERITED(BlobURLChannel, nsBaseChannel, nsIBlobURLChannel) >+ > BlobURLChannel::BlobURLChannel(nsIURI* aURI, > nsILoadInfo* aLoadInfo) > : mInitialized(false) > { > SetURI(aURI); > SetOriginalURI(aURI); > SetLoadInfo(aLoadInfo); > >@@ -30,17 +32,17 @@ void > BlobURLChannel::InitFailed() > { > MOZ_ASSERT(!mInitialized); > MOZ_ASSERT(!mInputStream); > mInitialized = true; > } > > void >-BlobURLChannel::Initialize(BlobImpl* aBlobImpl) >+BlobURLChannel::Initialize(BlobImpl* aBlobImpl, nsIPrincipal* aPrincipal) > { > MOZ_ASSERT(!mInitialized); > > nsAutoString contentType; > aBlobImpl->GetType(contentType); > SetContentType(NS_ConvertUTF16toUTF8(contentType)); > > if (aBlobImpl->IsFile()) { >@@ -60,16 +62,18 @@ BlobURLChannel::Initialize(BlobImpl* aBl > > aBlobImpl->CreateInputStream(getter_AddRefs(mInputStream), rv); > if (NS_WARN_IF(rv.Failed())) { > InitFailed(); > return; > } > > MOZ_ASSERT(mInputStream); >+ >+ mPrincipal = aPrincipal; > mInitialized = true; > } > > nsresult > BlobURLChannel::OpenContentStream(bool aAsync, nsIInputStream** aResult, > nsIChannel** aChannel) > { > MOZ_ASSERT(mInitialized); >@@ -85,9 +89,16 @@ BlobURLChannel::OpenContentStream(bool a > > return NS_OK; > } > > void > BlobURLChannel::OnChannelDone() > { > mInputStream = nullptr; >+ mPrincipal = nullptr; > } >+ >+nsIPrincipal* >+BlobURLChannel::GetPrincipal() const >+{ >+ return mPrincipal; >+} >diff --git a/dom/file/uri/BlobURLChannel.h b/dom/file/uri/BlobURLChannel.h >--- a/dom/file/uri/BlobURLChannel.h >+++ b/dom/file/uri/BlobURLChannel.h >@@ -13,40 +13,65 @@ > > class nsIURI; > > namespace mozilla { > namespace dom { > > class BlobImpl; > >-class BlobURLChannel final : public nsBaseChannel >+#define BLOBURLCHANNEL_IID \ >+ { 0x128959c3, 0xf00d, 0x436b, \ >+ { 0xa2, 0xba, 0x4b, 0xd5, 0x45, 0x59, 0x04, 0x69 } } >+ >+class nsIBlobURLChannel : public nsISupports > { > public: >+ NS_DECLARE_STATIC_IID_ACCESSOR(BLOBURLCHANNEL_IID) >+ >+ virtual nsIPrincipal* >+ GetPrincipal() const = 0; >+}; I don't see reason for nsIBlobURLChannel. BlobURLChannel could have IID itself, similar to dom::Element and such > >+ // The principal of the Blob's parent. parent? What is Blob's parent? >+ nsCOMPtr<nsIPrincipal> principal; >+ >+ // For revoked blobURLs, we use a null principal. >+ if (blobURL->Revoked()) { >+ principal = NullPrincipal::CreateWithoutOriginAttributes(); >+ } >+ >+ // If it's a known blobURL, let's use the known principal. >+ if (!principal) { >+ DataInfo* info = GetDataInfoFromURI(aURI, true /*aAlsoIfRevoked */); >+ if (info) { >+ // We know that this is something else. >+ if (info->mObjectType != DataInfo::eBlobImpl || !info->mBlobImpl) { >+ return false; >+ } >+ >+ principal = info->mPrincipal; >+ } >+ } >+ >+ if (principal) { >+ if (aOwnerURI) { >+ rv = principal->GetURI(aOwnerURI); So here ownerURI can be also the null principal uri, but in the part 1 it wouldn't be. >- if (aPrincipal) { >- principal.forget(aPrincipal); >+ // 'null/<uuid>' blobURLs should not leave the owning process. >+ if (StringBeginsWith(path, NS_LITERAL_CSTRING("null/"))) { >+ return false; Why we do string comparison and not check whether it is a null principal?
Attachment #9004513 - Flags: review?(bugs) → review-
Attachment #9004683 - Flags: review?(bugs) → review+
Comment on attachment 9004690 [details] [diff] [review] part 5 - Retrieve blobURL from the parent process >+void >+BlobURLChannel::InitializeRemote() >+{ >+ MOZ_ASSERT(!XRE_IsParentProcess()); >+ >+ nsCOMPtr<nsIURI> uri; >+ nsresult rv = GetURI(getter_AddRefs(uri)); >+ if (NS_WARN_IF(NS_FAILED(rv))) { >+ InitFailed(); >+ return; >+ } >+ >+ RefPtr<BlobURL> blobURL; >+ rv = uri->QueryInterface(kHOSTOBJECTURICID, getter_AddRefs(blobURL)); Huh, why is CID use for this. Could you file a followup to make BlobURL to have a normal IID >+void >+BlobURLChannel::SetPrincipal(nsIPrincipal* aPrincipal) >+{ >+ MOZ_ASSERT(aPrincipal && !mPrincipal); >+ MOZ_ASSERT_IF(mLoadInfo && >+ !nsContentUtils::IsSystemPrincipal(mLoadInfo->LoadingPrincipal()), >+ ChromeUtils::IsOriginAttributesEqualIgnoringFPD(mLoadInfo->GetOriginAttributes(), >+ BasePrincipal::Cast(aPrincipal)->OriginAttributesRef())); Why ignoring FPD? >+void >+BlobURLInputStream::DataReceived(BlobImpl* aBlobImpl, >+ nsIPrincipal* aPrincipal) DataReceived sounds a bit weird, when we receive BlobImpl. And then looks like this can be called with null params. Could this be something like... UpdateStreamContent? Not too good name either though. >+ if (hasAsyncWaitCallback) { >+ if (mInputStream) { >+ mInputStream->AsyncWait(this, asyncWaitFlags, asyncWaitRequestedCount, >+ asyncWaitTarget); >+ } else { >+ RefPtr<BlobURLInputStream> self = this; >+ nsCOMPtr<nsIRunnable> r = >+ NS_NewRunnableFunction("BlobURLInputStream::DataReceived", [self]() { >+ nsCOMPtr<nsIInputStreamCallback> callback; >+ { >+ MutexAutoLock lock(self->mMutex); >+ callback = self->mAsyncWaitCallback; >+ } >+ >+ if (callback) { >+ callback->OnInputStreamReady(self); >+ } >+ }); >+ >+ if (asyncWaitTarget) { >+ asyncWaitTarget->Dispatch(r, NS_DISPATCH_NORMAL); >+ } else { >+ r->Run(); >+ } >+ } >+ } >+} This needs some comment what is happening. What is the runnable function doing and why? >+class BlobURLInputStream final : public nsIAsyncInputStream >+ , public nsIInputStreamLength >+ , public nsIAsyncInputStreamLength >+ , public nsIInputStreamCallback >+{ >+public: >+ NS_DECL_THREADSAFE_ISUPPORTS >+ NS_DECL_NSIINPUTSTREAM >+ NS_DECL_NSIASYNCINPUTSTREAM >+ NS_DECL_NSIINPUTSTREAMLENGTH >+ NS_DECL_NSIASYNCINPUTSTREAMLENGTH >+ NS_DECL_NSIINPUTSTREAMCALLBACK >+ >+ static already_AddRefed<nsIInputStream> >+ Create(BlobURLChannel* aChannel, BlobURL* aURI); >+ >+ void >+ DataReceived(BlobImpl* aBlobImpl, >+ nsIPrincipal* aPrincipal); >+ >+private: >+ explicit BlobURLInputStream(BlobURLChannel* aChannel); >+ ~BlobURLInputStream(); >+ >+ Mutex mMutex; What all is protected by this mutex. Needs some documentation. >+ >+ enum State { { goes to its own line. >+ ePending, >+ ePendingClosed, >+ eReady, >+ eError, >+ }; These states need documentation. >@@ -836,16 +844,19 @@ private: > nsCOMPtr<nsIFile> mProfileDir; > #endif > > // Hashtable to keep track of the pending GetFilesHelper objects. > // This GetFilesHelperChild objects are removed when RecvGetFilesResponse is > // received. > nsRefPtrHashtable<nsIDHashKey, GetFilesHelperChild> mGetFilesPendingRequests; > >+ // Hashtable to keep track of the pending BlobURLInputStream objects. >+ nsRefPtrHashtable<nsIDHashKey, BlobURLInputStream> mBlobURLPendingStreams; Perhaps mPendingBlobURLStreams would sound a tad better. I'll need to read this several times. And we need quite a few tests for all this stuff.
Attachment #9004690 - Flags: review?(bugs) → review-
This is actually a fission blocker because it's about proactively broadcast data; rather than a rogue content process requesting it.
Blocks: fission
No longer blocks: fission-ipc
baku, are you planning on picking this back up? We're seeing crashes related to it over in bug 1513096 and it's definitely important for the content memshrink project.
Flags: needinfo?(amarchesini)
This issue will be resolved indirectly by fission project. We currently broadcast blobURLs, because, a process can load them at any time. When we load them, we need to extract the owning principal synchronously (See principal CheckMayLoad and subsumes methods). But, after fission, we don't need to broadcast them anymore because only the owning origin is allowed to open its own blobURLs and, all of this will be running on the same process. farre, Is this correct?
Flags: needinfo?(amarchesini) → needinfo?(afarre)

That should be the case.

Flags: needinfo?(afarre)

Baku, what is the current state of your BlobURL patches here?

We're tracking this bug as a potential blocker for Fission Nightly (M6).

Fission Milestone: --- → M6
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/26694602791a Broadcast BlobURLs only to processes with the same loaded origins, r=farre

https://hg.mozilla.org/mozilla-central/rev/26694602791a

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox73: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Flags: needinfo?(amarchesini)
Regressions: 1583000

The implementation that landed seems to only pay attention to window clients, and since ServiceWorkers are intentionally randomly allocated across all eligible processes, the test fails with high probability with a NetworkError when fetch fails to map the blob URL. (If ServiceWorkers used the same process-selection affinity as SharedWorkers, the test would have passed, but the patch would not handle SharedWorkers or ServiceWorkers correctly.)

The ClientManagerService already knows about all clients (which includes ServiceWorkers), and would be a better source of the information.

Note that I think there is also inherently a PContent/PBackground race in the broadcast mechanism given that the test already has some expected failures per https://searchfox.org/mozilla-central/source/testing/web-platform/meta/service-workers/service-worker/postmessage-blob-url.https.html.ini. (That is, the blob URL broadcast is sent over PContent, but the postMessage in the test goes via PBackground.)

Attachment #9116721 - Attachment description: Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre → Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre, r=asuth
Attachment #9116721 - Attachment description: Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre, r=asuth → Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre,asuth

As a follow-up to my comment 34, baku and I discussed the options via slack and decided that expanding the existing patch approach to also be aware of RemoteWorkers was the most pragmatic solution. (Especially given that fission largely moots the entire problem space.)

The ClientManagerService and Clients API exist on PBackground and the IPC actors exist on a per-global basis, when the semantics we want are really per-process. That would suggest creating something like the oft-discussed background-service-in-the-child mechanism which the "DOM File" thread and RemoteWorkerService and its "Worker Launcher" thread approximate. However, even that complexity would still run into propagation races between PContent and PBackground (via multiple paths). Using the Clients API and making sure that the blobs are relayed to every top-level actor at least once (and therefore all PBackground connection pairs between content threads and PBackground in the parent) in addition to PContent would address the problem, but creates coordination problems from receiving N updates. Plus, it massively multiplies the impact of a badly behaving site.

Also clearing the needinfo for :baku since the problem is understood and addressed and a fixed patch is up.

Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b9740bd7bf8f Broadcast BlobURLs only to processes with the same loaded origins, r=farre,asuth

https://hg.mozilla.org/mozilla-central/rev/b9740bd7bf8f

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox74: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Regressions: 1620921
Regressions: 1626573

Hi Adrian, as this bug is already resolved as FIXED, what does this dependency to bug 1636823 actually mean? (I interpret a dependency as "something that needs to be done before we can complete this work here" - which already has been completed in the past in this case.) Should we reopen this one? I'd prefer to just fix bug 1636823 as a normal defect of something we believed to work some time in the past.

If the intent is to say, the feature we implemented here once is now broken, I fear we have no consistent handling of "enhancement"s being "identifiable features" we could track defects against. I'd say, the most fine grained level we have for this in a quite consistent manner is the component itself (which we don't need to track as dependency, anyway) ?

Maybe a "see also" relation is more appropriate here to provide the wanted context? Am I missing something?

Flags: needinfo?(aflorinescu)

So basically the block means that this fix introduced bug 1636823, which I'm uncertain if it is a regression or it is intended behavior. If it's a regression, then bug 1636823 will be treated separately from this one and fixed accordingly. Even If it's not a regression, but a follow-up, then it will also be treated separately.
Let's wait for :baku to weight in on bug 1636823 as this bug will remain fixed regardless of the resolution of bug 1636823.

Flags: needinfo?(aflorinescu)
No longer depends on: 1636823
Regressions: 1636823
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: