Bug 1986533 - Improve font face error reporting. r=layout-reviewers,firefox-style-system-reviewers,dshin a=RyanVM DONTBUILD

emilio · rvandermeulen · commit 2dd6c5e57047 · 2025-09-26T20:06:44.000Z
With this patch, something like: let ff = new FontFace("content:Segoe", "https://foo.com"); ff.load(); Generates an exception like: > Uncaught (in promise) DOMException: Invalid font descriptor font-family: content:Segoe Rather than the error in the bug.
diff --git a/layout/style/FontFace.cpp b/layout/style/FontFace.cpp
@@ -24,8 +24,7 @@
 #include "mozilla/dom/UnionTypes.h"
 #include "nsStyleUtil.h"
 
-namespace mozilla {
-namespace dom {
+namespace mozilla::dom {
 
 // -- Utility functions ------------------------------------------------------
 
@@ -71,9 +70,7 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(FontFace)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(FontFace)
 
-FontFace::FontFace(nsIGlobalObject* aParent) : mLoadedRejection(NS_OK) {
-  BindToOwner(aParent);
-}
+FontFace::FontFace(nsIGlobalObject* aParent) { BindToOwner(aParent); }
 
 FontFace::~FontFace() {
   // Assert that we don't drop any FontFace objects during a Servo traversal,
@@ -285,20 +282,29 @@ void FontFace::MaybeResolve() {
   mLoaded->MaybeResolve(this);
 }
 
-void FontFace::MaybeReject(nsresult aResult) {
+void FontFace::MaybeReject(FontFaceLoadedRejectReason aReason,
+                           nsCString&& aMessage) {
   gfxFontUtils::AssertSafeThreadOrServoFontMetricsLocked();
 
   if (ServoStyleSet* ss = gfxFontUtils::CurrentServoStyleSet()) {
     // See comments in Gecko_GetFontMetrics.
-    ss->AppendTask(
-        PostTraversalTask::RejectFontFaceLoadedPromise(this, aResult));
+    ss->AppendTask(PostTraversalTask::RejectFontFaceLoadedPromise(
+        this, aReason, std::move(aMessage)));
     return;
   }
 
   if (mLoaded) {
-    mLoaded->MaybeReject(aResult);
-  } else if (mLoadedRejection == NS_OK) {
-    mLoadedRejection = aResult;
+    switch (aReason) {
+      case FontFaceLoadedRejectReason::Network:
+        mLoaded->MaybeRejectWithNetworkError(aMessage);
+        break;
+      case FontFaceLoadedRejectReason::Syntax:
+        mLoaded->MaybeRejectWithSyntaxError(aMessage);
+        break;
+    }
+  } else if (!mLoadedRejection) {
+    mLoadedRejection = MakeUnique<FontFaceLoadedRejection>(
+        FontFaceLoadedRejection{aReason, std::move(aMessage)});
   }
 }
 
@@ -307,15 +313,14 @@ void FontFace::EnsurePromise() {
     return;
   }
 
-  ErrorResult rv;
-  mLoaded = Promise::Create(GetOwnerGlobal(), rv);
+  mLoaded = Promise::CreateInfallible(GetOwnerGlobal());
 
   if (mImpl->Status() == FontFaceLoadStatus::Loaded) {
     mLoaded->MaybeResolve(this);
-  } else if (mLoadedRejection != NS_OK) {
-    mLoaded->MaybeReject(mLoadedRejection);
+  } else if (mLoadedRejection) {
+    auto rejection = std::move(mLoadedRejection);
+    MaybeReject(rejection->mReason, std::move(rejection->mMessage));
   }
 }
 
-}  // namespace dom
-}  // namespace mozilla
+}  // namespace mozilla::dom
diff --git a/layout/style/FontFace.h b/layout/style/FontFace.h
@@ -30,10 +30,12 @@ class FontFaceSet;
 class FontFaceSetImpl;
 class Promise;
 class UTF8StringOrArrayBufferOrArrayBufferView;
-}  // namespace dom
-}  // namespace mozilla
 
-namespace mozilla::dom {
+enum class FontFaceLoadedRejectReason : uint8_t { Syntax, Network };
+struct FontFaceLoadedRejection {
+  const FontFaceLoadedRejectReason mReason;
+  nsCString mMessage;
+};
 
 class FontFace final : public GlobalTeardownObserver, public nsWrapperCache {
   friend class mozilla::PostTraversalTask;
@@ -92,7 +94,8 @@ class FontFace final : public GlobalTeardownObserver, public nsWrapperCache {
 
   void Destroy();
   void MaybeResolve();
-  void MaybeReject(nsresult aResult);
+
+  void MaybeReject(FontFaceLoadedRejectReason, nsCString&& aRejectMessage);
 
  private:
   explicit FontFace(nsIGlobalObject* aParent);
@@ -115,9 +118,10 @@ class FontFace final : public GlobalTeardownObserver, public nsWrapperCache {
   RefPtr<Promise> mLoaded;
 
   // Saves the rejection code for mLoaded if mLoaded hasn't been created yet.
-  nsresult mLoadedRejection;
+  UniquePtr<FontFaceLoadedRejection> mLoadedRejection;
 };
 
-}  // namespace mozilla::dom
+}  // namespace dom
+}  // namespace mozilla
 
 #endif  // !defined(mozilla_dom_FontFace_h)
diff --git a/layout/style/FontFaceImpl.cpp b/layout/style/FontFaceImpl.cpp
@@ -16,8 +16,7 @@
 #include "mozilla/dom/FontFaceBinding.h"
 #include "mozilla/dom/FontFaceSetImpl.h"
 
-namespace mozilla {
-namespace dom {
+namespace mozilla::dom {
 
 // -- FontFaceBufferSource ---------------------------------------------------
 
@@ -122,7 +121,9 @@ void FontFaceImpl::InitializeSourceURL(const nsACString& aURL) {
   IgnoredErrorResult rv;
   SetDescriptor(eCSSFontDesc_Src, aURL, rv);
   if (rv.Failed()) {
-    mOwner->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
+    mOwner->MaybeReject(FontFaceLoadedRejectReason::Syntax,
+                        nsPrintfCString("Invalid source url %s",
+                                        PromiseFlatCString(aURL).get()));
     SetStatus(FontFaceLoadStatus::Error);
   }
 }
@@ -402,9 +403,10 @@ void FontFaceImpl::UpdateOwnerPromise() {
     mOwner->MaybeResolve();
   } else if (mStatus == FontFaceLoadStatus::Error) {
     if (mSourceType == eSourceType_Buffer) {
-      mOwner->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
+      mOwner->MaybeReject(FontFaceLoadedRejectReason::Syntax,
+                          nsCString("Invalid source buffer"_ns));
     } else {
-      mOwner->MaybeReject(NS_ERROR_DOM_NETWORK_ERR);
+      mOwner->MaybeReject(FontFaceLoadedRejectReason::Network, nsCString());
     }
   }
 
@@ -443,7 +445,10 @@ bool FontFaceImpl::SetDescriptor(nsCSSFontDesc aFontDesc,
   bool changed;
   if (!Servo_FontFaceRule_SetDescriptor(GetData(), aFontDesc, &aValue, url,
                                         &changed)) {
-    aRv.ThrowSyntaxError("Invalid font descriptor");
+    aRv.ThrowSyntaxError(
+        nsPrintfCString("Invalid font descriptor %s: %s",
+                        nsCSSProps::GetStringValue(aFontDesc).get(),
+                        PromiseFlatCString(aValue).get()));
     return false;
   }
 
@@ -465,11 +470,18 @@ bool FontFaceImpl::SetDescriptors(const nsACString& aFamily,
 
   mDescriptors = Servo_FontFaceRule_CreateEmpty().Consume();
 
+  nsCString errorMessage;
   // Helper to call SetDescriptor and return true on success, false on failure.
-  auto setDesc = [=](nsCSSFontDesc aDesc, const nsACString& aVal) -> bool {
+  auto setDesc = [&](nsCSSFontDesc aDesc, const nsACString& aVal) -> bool {
     IgnoredErrorResult rv;
     SetDescriptor(aDesc, aVal, rv);
-    return !rv.Failed();
+    if (!rv.Failed()) {
+      return true;
+    }
+    errorMessage = nsPrintfCString("Invalid font descriptor %s: %s",
+                                   nsCSSProps::GetStringValue(aDesc).get(),
+                                   PromiseFlatCString(aVal).get());
+    return false;
   };
 
   // Parse all of the mDescriptors in aInitializer, which are the values
@@ -497,7 +509,8 @@ bool FontFaceImpl::SetDescriptors(const nsACString& aFamily,
     mDescriptors = Servo_FontFaceRule_CreateEmpty().Consume();
 
     if (mOwner) {
-      mOwner->MaybeReject(NS_ERROR_DOM_SYNTAX_ERR);
+      mOwner->MaybeReject(FontFaceLoadedRejectReason::Syntax,
+                          std::move(errorMessage));
     }
 
     SetStatus(FontFaceLoadStatus::Error);
@@ -860,5 +873,4 @@ void FontFaceImpl::Entry::RemoveFontFace(FontFaceImpl* aFontFace) {
   CheckUserFontSetLocked();
 }
 
-}  // namespace dom
-}  // namespace mozilla
+}  // namespace mozilla::dom
diff --git a/layout/style/PostTraversalTask.cpp b/layout/style/PostTraversalTask.cpp
@@ -25,7 +25,8 @@ void PostTraversalTask::Run() {
       break;
 
     case Type::RejectFontFaceLoadedPromise:
-      static_cast<FontFace*>(mTarget)->MaybeReject(mResult);
+      static_cast<FontFace*>(mTarget)->MaybeReject(mResult.extract(),
+                                                   std::move(mMessage));
       break;
 
     case Type::DispatchLoadingEventAndReplaceReadyPromise:
diff --git a/layout/style/PostTraversalTask.h b/layout/style/PostTraversalTask.h
@@ -7,13 +7,15 @@
 #ifndef mozilla_PostTraversalTask_h
 #define mozilla_PostTraversalTask_h
 
+#include "nsString.h"
 #include "nscore.h"
 
 /* a task to be performed immediately after a Servo traversal */
 
 namespace mozilla {
 class ServoStyleSet;
 namespace dom {
+enum class FontFaceLoadedRejectReason : uint8_t;
 class FontFace;
 class FontFaceSet;
 class FontFaceSetImpl;
@@ -43,11 +45,13 @@ class PostTraversalTask {
     return task;
   }
 
-  static PostTraversalTask RejectFontFaceLoadedPromise(dom::FontFace* aFontFace,
-                                                       nsresult aResult) {
+  static PostTraversalTask RejectFontFaceLoadedPromise(
+      dom::FontFace* aFontFace, dom::FontFaceLoadedRejectReason aReason,
+      nsCString&& aMessage) {
     auto task = PostTraversalTask(Type::ResolveFontFaceLoadedPromise);
     task.mTarget = aFontFace;
-    task.mResult = aResult;
+    task.mResult.emplace(aReason);
+    task.mMessage = std::move(aMessage);
     return task;
   }
 
@@ -96,7 +100,7 @@ class PostTraversalTask {
     ResolveFontFaceLoadedPromise,
 
     // mTarget (FontFace*)
-    // mResult
+    // mResult / mMessage
     RejectFontFaceLoadedPromise,
 
     // mTarget (FontFaceSet*)
@@ -115,12 +119,12 @@ class PostTraversalTask {
     FontInfoUpdate,
   };
 
-  explicit PostTraversalTask(Type aType)
-      : mType(aType), mTarget(nullptr), mResult(NS_OK) {}
+  explicit PostTraversalTask(Type aType) : mType(aType) {}
 
-  Type mType;
-  void* mTarget;
-  nsresult mResult;
+  const Type mType;
+  void* mTarget = nullptr;
+  nsCString mMessage;
+  Maybe<dom::FontFaceLoadedRejectReason> mResult;
 };
 
 }  // namespace mozilla
