Bug 1695490 - Remove svg.text-spacing.enabled pref try: -b d -p linux64 -u reftest,crashtest -t none draft bug1695490
authorlongsonr <longsonr@gmail.com>
Sun, 28 Feb 2021 12:13:03 +0000
changeset 3573461 a38ef7b6f362e461b5516303e349d07dcb492fbd
parent 3546204 6d7590bfd8d37fd1088f8d184702238881fc048b
child 3699002 ad936fbea453ff5f1339adcf2c7dd1301b2ca8ec
push id661283
push userlongsonr@gmail.com
push dateSun, 28 Feb 2021 12:20:26 +0000
treeherdertry@a38ef7b6f362 [default view] [failures only]
bugs1695490, 1599173, 1600855
milestone87.0a1
Bug 1695490 - Remove svg.text-spacing.enabled pref try: -b d -p linux64 -u reftest,crashtest -t none Summary: Backs out bug 1599173 which landed in Firefox 72. The pref has been enabled since bug 1600855 which landed in Firefox 73 Reviewers: emilio Tags: #secure-revision Bug #: 1695490 Differential Revision: https://phabricator.services.mozilla.com/D106731
layout/generic/nsTextFrame.cpp
layout/reftests/svg/text/reftest.list
layout/svg/SVGTextFrame.cpp
modules/libpref/init/StaticPrefList.yaml
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -14,17 +14,16 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/StaticPrefs_layout.h"
-#include "mozilla/StaticPrefs_svg.h"
 #include "mozilla/StaticPresData.h"
 #include "mozilla/SVGTextFrame.h"
 #include "mozilla/SVGUtils.h"
 #include "mozilla/TextEditor.h"
 #include "mozilla/TextEvents.h"
 #include "mozilla/BinarySearch.h"
 #include "mozilla/IntegerRange.h"
 #include "mozilla/Unused.h"
@@ -1768,19 +1767,16 @@ static float GetSVGFontSizeScaleFactor(n
 
 static nscoord LetterSpacing(nsIFrame* aFrame,
                              const nsStyleText* aStyleText = nullptr) {
   if (!aStyleText) {
     aStyleText = aFrame->StyleText();
   }
 
   if (SVGUtils::IsInSVGTextSubtree(aFrame)) {
-    if (!StaticPrefs::svg_text_spacing_enabled()) {
-      return 0;
-    }
     // SVG text can have a scaling factor applied so that very small or very
     // large font-sizes don't suffer from poor glyph placement due to app unit
     // rounding. The used letter-spacing value must be scaled by the same
     // factor.
     Length spacing = aStyleText->mLetterSpacing;
     spacing.ScaleBy(GetSVGFontSizeScaleFactor(aFrame));
     return spacing.ToAppUnits();
   }
@@ -1797,37 +1793,29 @@ static nscoord WordSpacing(nsIFrame* aFr
 
   if (SVGUtils::IsInSVGTextSubtree(aFrame)) {
     // SVG text can have a scaling factor applied so that very small or very
     // large font-sizes don't suffer from poor glyph placement due to app unit
     // rounding. The used word-spacing value must be scaled by the same
     // factor, although any percentage basis has already effectively been
     // scaled, since it's the space glyph width, which is based on the already-
     // scaled font-size.
-    if (!StaticPrefs::svg_text_spacing_enabled()) {
-      return 0;
-    }
     auto spacing = aStyleText->mWordSpacing;
     spacing.ScaleLengthsBy(GetSVGFontSizeScaleFactor(aFrame));
     return spacing.Resolve([&] { return GetSpaceWidthAppUnits(aTextRun); });
   }
 
   return aStyleText->mWordSpacing.Resolve(
       [&] { return GetSpaceWidthAppUnits(aTextRun); });
 }
 
 // Returns gfxTextRunFactory::TEXT_ENABLE_SPACING if non-standard
 // letter-spacing or word-spacing is present.
 static gfx::ShapedTextFlags GetSpacingFlags(
     nsIFrame* aFrame, const nsStyleText* aStyleText = nullptr) {
-  if (SVGUtils::IsInSVGTextSubtree(aFrame) &&
-      !StaticPrefs::svg_text_spacing_enabled()) {
-    return gfx::ShapedTextFlags();
-  }
-
   const nsStyleText* styleText = aFrame->StyleText();
   const auto& ls = styleText->mLetterSpacing;
   const auto& ws = styleText->mWordSpacing;
 
   // It's possible to have a calc() value that computes to zero but for which
   // IsDefinitelyZero() is false, in which case we'll return
   // TEXT_ENABLE_SPACING unnecessarily. That's ok because such cases are likely
   // to be rare, and avoiding TEXT_ENABLE_SPACING is just an optimization.
--- a/layout/reftests/svg/text/reftest.list
+++ b/layout/reftests/svg/text/reftest.list
@@ -203,11 +203,11 @@ random-if(/^Windows\x20NT\x206\.1/.test(
 fuzzy-if(skiaContent,0-1,0-100) needs-focus fuzzy-if(webrender&&winWidget,134-148,261-318) == simple-bidi-selection.svg simple-bidi-selection-ref.html
 fuzzy-if(skiaContent,0-1,0-50) needs-focus fuzzy-if(webrender&&winWidget,127-148,221-254) fuzzy-if(webrender&&OSX,1-65,19-196) == simple-fill-color-selection.svg simple-fill-color-selection-ref.html
 fuzzy-if(skiaContent,0-1,0-150) needs-focus fuzzy-if(webrender&&winWidget,125-148,221-254) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == simple-underline-selection.svg simple-underline-selection-ref.html # Bug 1392106
 fuzzy-if(skiaContent,0-1,0-300) needs-focus fuzzy-if(webrender&&winWidget,134-152,432-501) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == multiple-text-selection.svg multiple-text-selection-ref.html # Bug 1392106
 needs-focus == multiple-chunks-selection.svg multiple-chunks-selection-ref.svg
 fuzzy-if(skiaContent,0-1,0-200) needs-focus == textpath-selection.svg textpath-selection-ref.svg
 
 # letter-spacing and word-spacing
-pref(svg.text-spacing.enabled,true) == simple-letter-spacing.svg simple-letter-spacing-ref.svg
-pref(svg.text-spacing.enabled,true) == simple-word-spacing.svg simple-word-spacing-ref.svg
-pref(svg.text-spacing.enabled,true) == multiple-chunks-letter-spacing.svg multiple-chunks-letter-spacing-ref.svg
+== simple-letter-spacing.svg simple-letter-spacing-ref.svg
+== simple-word-spacing.svg simple-word-spacing-ref.svg
+== multiple-chunks-letter-spacing.svg multiple-chunks-letter-spacing-ref.svg
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -43,17 +43,16 @@
 #include "mozilla/dom/Selection.h"
 #include "mozilla/dom/SVGGeometryElement.h"
 #include "mozilla/dom/SVGRect.h"
 #include "mozilla/dom/SVGTextContentElementBinding.h"
 #include "mozilla/dom/SVGTextPathElement.h"
 #include "mozilla/dom/Text.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/PatternHelpers.h"
-#include "mozilla/StaticPrefs_svg.h"
 #include <algorithm>
 #include <cmath>
 #include <limits>
 
 using namespace mozilla::dom;
 using namespace mozilla::dom::SVGTextContentElement_Binding;
 using namespace mozilla::gfx;
 using namespace mozilla::image;
@@ -826,37 +825,34 @@ SVGBBox TextRenderedRun::GetRunUserSpace
                                          mTextFrameContentLength);
   if (range.Length() == 0) {
     return r;
   }
 
   // FIXME(heycam): We could create a single PropertyProvider for all
   // TextRenderedRuns that correspond to the text frame, rather than recreate
   // it each time here.
-  Maybe<nsTextFrame::PropertyProvider> provider;
-  if (StaticPrefs::svg_text_spacing_enabled()) {
-    provider.emplace(mFrame, start);
-  }
+  nsTextFrame::PropertyProvider provider(mFrame, start);
 
   // Measure that range.
   gfxTextRun::Metrics metrics = textRun->MeasureText(
-      range, gfxFont::LOOSE_INK_EXTENTS, nullptr, provider.ptrOr(nullptr));
+      range, gfxFont::LOOSE_INK_EXTENTS, nullptr, &provider);
   // Make sure it includes the font-box.
   gfxRect fontBox(0, -metrics.mAscent, metrics.mAdvanceWidth,
                   metrics.mAscent + metrics.mDescent);
   metrics.mBoundingBox.UnionRect(metrics.mBoundingBox, fontBox);
 
   // Determine the rectangle that covers the rendered run's fill,
   // taking into account the measured vertical overflow due to
   // decorations.
   nscoord baseline = metrics.mBoundingBox.y + metrics.mAscent;
   gfxFloat x, width;
   if (aFlags & eNoHorizontalOverflow) {
     x = 0.0;
-    width = textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr));
+    width = textRun->GetAdvanceWidth(range, &provider);
   } else {
     x = metrics.mBoundingBox.x;
     width = metrics.mBoundingBox.width;
   }
   nsRect fillInAppUnits(x, baseline - above, width,
                         metrics.mBoundingBox.height + above + below);
   if (textRun->IsVertical()) {
     // Swap line-relative textMetrics dimensions to physical coordinates.
@@ -927,20 +923,17 @@ void TextRenderedRun::GetClipEdges(nscoo
     // to clip without having to measure anything.
     aVisIStartEdge = 0;
     aVisIEndEdge = 0;
     return;
   }
 
   gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
   gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
-  Maybe<nsTextFrame::PropertyProvider> provider;
-  if (StaticPrefs::svg_text_spacing_enabled()) {
-    provider.emplace(mFrame, it);
-  }
+  nsTextFrame::PropertyProvider provider(mFrame, it);
 
   // Get the covered content offset/length for this rendered run in skipped
   // characters, since that is what GetAdvanceWidth expects.
   Range runRange = ConvertOriginalToSkipped(it, mTextFrameContentOffset,
                                             mTextFrameContentLength);
 
   // Get the offset/length of the whole nsTextFrame.
   uint32_t frameOffset = mFrame->GetContentOffset();
@@ -955,44 +948,41 @@ void TextRenderedRun::GetClipEdges(nscoo
 
   // Convert the trimmed whole-nsTextFrame offset/length into skipped
   // characters.
   Range frameRange = ConvertOriginalToSkipped(it, frameOffset, frameLength);
 
   // Measure the advance width in the text run between the start of
   // frame's content and the start of the rendered run's content,
   nscoord startEdge = textRun->GetAdvanceWidth(
-      Range(frameRange.start, runRange.start), provider.ptrOr(nullptr));
+      Range(frameRange.start, runRange.start), &provider);
 
   // and between the end of the rendered run's content and the end
   // of the frame's content.
-  nscoord endEdge = textRun->GetAdvanceWidth(
-      Range(runRange.end, frameRange.end), provider.ptrOr(nullptr));
+  nscoord endEdge =
+      textRun->GetAdvanceWidth(Range(runRange.end, frameRange.end), &provider);
 
   if (textRun->IsRightToLeft()) {
     aVisIStartEdge = endEdge;
     aVisIEndEdge = startEdge;
   } else {
     aVisIStartEdge = startEdge;
     aVisIEndEdge = endEdge;
   }
 }
 
 nscoord TextRenderedRun::GetAdvanceWidth() const {
   gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
   gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
-  Maybe<nsTextFrame::PropertyProvider> provider;
-  if (StaticPrefs::svg_text_spacing_enabled()) {
-    provider.emplace(mFrame, it);
-  }
+  nsTextFrame::PropertyProvider provider(mFrame, it);
 
   Range range = ConvertOriginalToSkipped(it, mTextFrameContentOffset,
                                          mTextFrameContentLength);
 
-  return textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr));
+  return textRun->GetAdvanceWidth(range, &provider);
 }
 
 int32_t TextRenderedRun::GetCharNumAtPosition(nsPresContext* aContext,
                                               const gfxPoint& aPoint) const {
   if (mTextFrameContentLength == 0) {
     return -1;
   }
 
@@ -1027,41 +1017,38 @@ int32_t TextRenderedRun::GetCharNumAtPos
     if (p.y < aContext->AppUnitsToGfxUnits(topEdge) ||
         p.y > aContext->AppUnitsToGfxUnits(bottomEdge)) {
       return -1;
     }
   }
 
   gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
   gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
-  Maybe<nsTextFrame::PropertyProvider> provider;
-  if (StaticPrefs::svg_text_spacing_enabled()) {
-    provider.emplace(mFrame, it);
-  }
+  nsTextFrame::PropertyProvider provider(mFrame, it);
 
   // Next check that the point lies horizontally within the left and right
   // edges of the text.
   Range range = ConvertOriginalToSkipped(it, mTextFrameContentOffset,
                                          mTextFrameContentLength);
-  gfxFloat runAdvance = aContext->AppUnitsToGfxUnits(
-      textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr)));
+  gfxFloat runAdvance =
+      aContext->AppUnitsToGfxUnits(textRun->GetAdvanceWidth(range, &provider));
 
   gfxFloat pos = writingMode.IsVertical() ? p.y : p.x;
   if (pos < 0 || pos >= runAdvance) {
     return -1;
   }
 
   // Finally, measure progressively smaller portions of the rendered run to
   // find which glyph it lies within.  This will need to change once we
   // support letter-spacing and word-spacing.
   bool rtl = textRun->IsRightToLeft();
   for (int32_t i = mTextFrameContentLength - 1; i >= 0; i--) {
     range = ConvertOriginalToSkipped(it, mTextFrameContentOffset, i);
     gfxFloat advance = aContext->AppUnitsToGfxUnits(
-        textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr)));
+        textRun->GetAdvanceWidth(range, &provider));
     if ((rtl && pos < runAdvance - advance) || (!rtl && pos >= advance)) {
       return i;
     }
   }
   return -1;
 }
 
 // ----------------------------------------------------------------------------
@@ -2317,24 +2304,21 @@ bool CharIterator::IsOriginalCharTrimmed
 }
 
 gfxFloat CharIterator::GetAdvance(nsPresContext* aContext) const {
   float cssPxPerDevPx =
       nsPresContext::AppUnitsToFloatCSSPixels(aContext->AppUnitsPerDevPixel());
 
   gfxSkipCharsIterator start =
       TextFrame()->EnsureTextRun(nsTextFrame::eInflated);
-  Maybe<nsTextFrame::PropertyProvider> provider;
-  if (StaticPrefs::svg_text_spacing_enabled()) {
-    provider.emplace(TextFrame(), start);
-  }
+  nsTextFrame::PropertyProvider provider(TextFrame(), start);
 
   uint32_t offset = mSkipCharsIterator.GetSkippedOffset();
-  gfxFloat advance = mTextRun->GetAdvanceWidth(Range(offset, offset + 1),
-                                               provider.ptrOr(nullptr));
+  gfxFloat advance =
+      mTextRun->GetAdvanceWidth(Range(offset, offset + 1), &provider);
   return aContext->AppUnitsToGfxUnits(advance) * mLengthAdjustScaleFactor *
          cssPxPerDevPx;
 }
 
 bool CharIterator::NextCharacter() {
   if (AtEnd()) {
     return false;
   }
@@ -3682,25 +3666,22 @@ float SVGTextFrame::GetSubStringLength(n
     IntersectInterval(offset, trimmedLength, charnum, nchars);
 
     if (trimmedLength != 0) {
       // Convert offset into an index into the frame.
       offset += trimmedOffset - textElementCharIndex;
 
       gfxSkipCharsIterator it = frame->EnsureTextRun(nsTextFrame::eInflated);
       gfxTextRun* textRun = frame->GetTextRun(nsTextFrame::eInflated);
-      Maybe<nsTextFrame::PropertyProvider> provider;
-      if (StaticPrefs::svg_text_spacing_enabled()) {
-        provider.emplace(frame, it);
-      }
+      nsTextFrame::PropertyProvider provider(frame, it);
 
       Range range = ConvertOriginalToSkipped(it, offset, trimmedLength);
 
       // Accumulate the advance.
-      textLength += textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr));
+      textLength += textRun->GetAdvanceWidth(range, &provider);
     }
 
     // Advance, ready for next call:
     frameStartTextElementCharIndex += untrimmedLength;
   }
 
   nsPresContext* presContext = PresContext();
   float cssPxPerDevPx = nsPresContext::AppUnitsToFloatCSSPixels(
@@ -3762,25 +3743,22 @@ float SVGTextFrame::GetSubStringLengthSl
 
     if (length != 0) {
       // Convert offset into an index into the frame.
       offset += run.mTextFrameContentOffset - run.mTextElementCharIndex;
 
       gfxSkipCharsIterator it =
           run.mFrame->EnsureTextRun(nsTextFrame::eInflated);
       gfxTextRun* textRun = run.mFrame->GetTextRun(nsTextFrame::eInflated);
-      Maybe<nsTextFrame::PropertyProvider> provider;
-      if (StaticPrefs::svg_text_spacing_enabled()) {
-        provider.emplace(run.mFrame, it);
-      }
+      nsTextFrame::PropertyProvider provider(run.mFrame, it);
 
       Range range = ConvertOriginalToSkipped(it, offset, length);
 
       // Accumulate the advance.
-      textLength += textRun->GetAdvanceWidth(range, provider.ptrOr(nullptr));
+      textLength += textRun->GetAdvanceWidth(range, &provider);
     }
 
     run = runIter.Next();
   }
 
   nsPresContext* presContext = PresContext();
   float cssPxPerDevPx = nsPresContext::AppUnitsToFloatCSSPixels(
       presContext->AppUnitsPerDevPixel());
@@ -4318,20 +4296,17 @@ void SVGTextFrame::DetermineCharPosition
   NS_ASSERTION(aPositions.IsEmpty(), "expected aPositions to be empty");
 
   nsPoint position;
 
   TextFrameIterator frit(this);
   for (nsTextFrame* frame = frit.Current(); frame; frame = frit.Next()) {
     gfxSkipCharsIterator it = frame->EnsureTextRun(nsTextFrame::eInflated);
     gfxTextRun* textRun = frame->GetTextRun(nsTextFrame::eInflated);
-    Maybe<nsTextFrame::PropertyProvider> provider;
-    if (StaticPrefs::svg_text_spacing_enabled()) {
-      provider.emplace(frame, it);
-    }
+    nsTextFrame::PropertyProvider provider(frame, it);
 
     // Reset the position to the new frame's position.
     position = frit.Position();
     if (textRun->IsVertical()) {
       if (textRun->IsRightToLeft()) {
         position.y += frame->GetRect().height;
       }
       position.x += GetBaselinePosition(frame, textRun, frit.DominantBaseline(),
@@ -4362,18 +4337,18 @@ void SVGTextFrame::DetermineCharPosition
       aPositions.AppendElement(position);
       if (!it.IsOriginalCharSkipped()) {
         // Accumulate partial ligature advance into position.  (We must get
         // partial advances rather than get the advance of the whole ligature
         // group / cluster at once, since the group may span text frames, and
         // the PropertyProvider only has spacing information for the current
         // text frame.)
         uint32_t offset = it.GetSkippedOffset();
-        nscoord advance = textRun->GetAdvanceWidth(Range(offset, offset + 1),
-                                                   provider.ptrOr(nullptr));
+        nscoord advance =
+            textRun->GetAdvanceWidth(Range(offset, offset + 1), &provider);
         (textRun->IsVertical() ? position.y : position.x) +=
             textRun->IsRightToLeft() ? -advance : advance;
       }
       it.AdvanceOriginal(1);
     }
   }
 
   // Finally any characters at the end that are not in a frame.
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -9775,22 +9775,16 @@
 
 # Is support for the new getBBox method from SVG 2 enabled?
 # See https://svgwg.org/svg2-draft/single-page.html#types-SVGBoundingBoxOptions
 - name: svg.new-getBBox.enabled
   type: bool
   value: false
   mirror: always
 
-# Is support for letter-spacing and word-spacing in SVG text enabled?
-- name: svg.text-spacing.enabled
-  type: bool
-  value: true
-  mirror: always
-
 #---------------------------------------------------------------------------
 # Prefs starting with "telemetry."
 #---------------------------------------------------------------------------
 
 # Enable origin telemetry test mode or not
 # NOTE: turning this on will override the
 #       privacy.trackingprotection.origin_telemetry.enabled pref.
 - name: telemetry.origin_telemetry_test_mode.enabled