1243734 - Have a pref to enable/ disable all new mask CSS properties added in bug 686281

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

According to bug 686281 comment 447, add a pref to enable all of the new work in bug 686281 when it's set to true, but make us support only what we supported before this patch when set to false

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

[Tracking Requested - why for this release]:  We either need to land this in this cycle in order to properly pref-off the feature that landed which is not ready to ship, or we need to back that feature out.

Comment 20

[u459114]

bug 1239441 comment 34 need to be addressed before this bug landed

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

https://reviewboard.mozilla.org/r/35159/#review32227

The separation between patches doesn't really seem to make sense here, but I'm not going to worry about it this time.

Please fix the spelling of "switch" in the commit message.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719888 [details]
MozReview Request: Bug 1243734 - Part 2. Use MOZ_ENABLE_MASK_AS_SHORTHAND to define the type of mask property; r=dbaron

https://reviewboard.mozilla.org/r/35161/#review32359

::: layout/style/nsCSSDataBlock.cpp:722
(Diff revision 3)
> -  CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID, aEnabledState) {
> +  CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID,
> +                                       (aEnabledState |
> +                                        nsCSSProps::eForceEnableMaskImage)) {

It seems to me like you want this change for all callers of CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES, not just the 3 that you modified in this patch.

It seems like instead of adding a new enabled flag and making these changes to the users of CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES, you could just change the CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES macro itself to explicitly check for mask_image after calling IsEnabled.

Even with that, though, this approach seems a little bit sketchy; I'm trying to think if there's a better way.

::: layout/style/nsCSSParser.cpp:11728
(Diff revision 3)
> +  nsCSSKeyword keyword = nsCSSKeywords::LookupKeyword(mToken.mIdent);
> +  nsCSSValueList* imageList = image.SetListValue();
> +  // At the moment mask been treated as longhand, it only accept VARIANT_HUO
> +  // typed value.
> +  if (keyword == eCSSKeyword_none) {
> +    if (ParseSingleValueProperty(imageList->mValue, eCSSProperty_mask_image) !=
> +          CSSParseResult::Ok) {
> +      NS_NOTREACHED("should be able to parse");
> +      return false;
> +    }
> +  } else if (mToken.mType == eCSSToken_URL) {
> +    if (ParseSingleValueProperty(imageList->mValue, eCSSProperty_mask_image) !=
> +          CSSParseResult::Ok) {
> +      return false;
> +    }
> +  } else {
> +    return false;
> +  }

You don't need this complexity.  Skip testing anything on keyword, and just call ParseVariant(imageList->mValue, VARIANT_URL | VARIANT_NONE, nullptr), return false if it wasn't CSSParseResult::OK, and then (otherwise) call AppendValue.

Comment 23

[u459114]

https://reviewboard.mozilla.org/r/35161/#review32359

> It seems to me like you want this change for all callers of CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES, not just the 3 that you modified in this patch.
> 
> It seems like instead of adding a new enabled flag and making these changes to the users of CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES, you could just change the CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES macro itself to explicitly check for mask_image after calling IsEnabled.
> 
> Even with that, though, this approach seems a little bit sketchy; I'm trying to think if there's a better way.

Since I didn't find another case that need shorthand to/from longhand runtime switching, so I choosed this simple solution, which work for mask only and change the least code.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The alternative approach would be to use #ifdefs for the "backend" half of the code, i.e., nsCSSPropList.h and things that fall out from it, while leaving the same structures in nsStyleStruct.  This would mean you'd have to write a little bit of new code in nsRuleNode to turn the single mask property into a one-item list with just a mask-image.

This seems like it's likely to be cleaner, and less risky in terms of missing things or introducing weird behaviors, although slightly more risky in terms of risk of compile-time bustage when merging.

It's hard to tell for sure without trying it, though.

Comment 25

[u459114]

Attachment: 0001-Bug-1243734-Part-1.-Add-MOZ_ENABLE_MASK_AS_SHORTHAND.patch

Comment 26

[u459114]

Attachment: 0002-Bug-1243734-Part-2.-Use-MOZ_ENABLE_MASK_AS_SHORTHAND.patch

Comment 27

[u459114]

Attachment: 0003-Bug-1243734-Part-3.-Fix-incorrect-rendering-result-w.patch

Comment 28

[u459114]

Attachment: 0004-Bug-1243734-Part-4.-Set-up-gCSSProperties-depends-on.patch

Comment 29

[u459114]

Attachment: 0003-Bug-1243734-Part-3.-Fix-incorrect-rendering-result-w.patch

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could you explain why Part 3 is needed?  Shouldn't part 2 have been done so that the structure is the same, and it's not needed?

Comment 31

[u459114]

(In reply to David Baron [:dbaron] ⌚️UTC-8 from comment #30)
> Could you explain why Part 3 is needed?  Shouldn't part 2 have been done so
> that the structure is the same, and it's not needed?

Short answer: we have a bug in nsSVGIntegrationUtils::PaintFramesWithEffects, change in part 2 bring this bug to the surface. 
And I am writing a longer explanation.

Comment 32

[u459114]

I catch this issue while testing https://dxr.mozilla.org/mozilla-central/source/layout/reftests/svg/mask-basic-01.svg

Let have a look on this if statement in nsSVGIntegrationUtils::PaintFramesWithEffects
if (svgMaskFrame) {
  // (a)
} else {
  // (b)
}

1. Before bug 686281, (b) dose not exist yet. If mask url is not resolvable, (a) will be skipped, so we do nothing at that moment.
2. After bug 686281, if mask url is not resolvable, (b) is executed.
3. After Part 2 of this bug, if mask url is not resolvable, (b) is executed.

#2: the type of svgReset->mMask.mLayers[i].mImage(which is nsStyleImage) is eStyleImageType_Image
If current document is a nsSVGDocument, this document will assign an empty vector image to svgReset->mMask.mLayers[i].mImage(which is nsStyleImage) even if the url path is wrong. We still have one mask image render onto target maskSurface. So the render result is _magically_ correct while testing the patches of bug 686281.

#3: the type of svgReset->mMask.mLayers[i].mImage is eStyleImageType_Null.
In Part2, nsRuleNode.cpp only set up mSourceURI and left mImage blank. Now we don’t have any mask image render onto maskSurface, left it totally transparent, which mean every pixels of background will be filter out.

Comment 33

[u459114]

Try result
Turning on mask-as-shorthand
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c60bc08d885
Turning off mask-as-shorthand
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e23689fffb68

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8722853 [details] [diff] [review]
0003-Bug-1243734-Part-3.-Fix-incorrect-rendering-result-w.patch

It sounds like you're saying that part 3 fixes an independent problem that just happened to not show up when you landed bug 686281.  Could you put that in its own bug report (blocking this one), explain the problem clearly there, and request review from an appropriate reviewer (which I don't think I am).

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8722849 [details] [diff] [review]
0001-Bug-1243734-Part-1.-Add-MOZ_ENABLE_MASK_AS_SHORTHAND.patch

I don't think you need or want a configure option; this is something that we should decide about.  You also don't need an AC_SUBST(), since all you use are #ifdefs in C++ files.  So you should just add a commented-out top-level AC_DEFINE() line somewhere to indicate that uncommenting it would enable the feature.

(This has now moved to old-configure.in.)

You probably want build peer review as well given how much configure is changing right now.

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8722850 [details] [diff] [review]
0002-Bug-1243734-Part-2.-Use-MOZ_ENABLE_MASK_AS_SHORTHAND.patch

>+#ifdef MOZ_ENABLE_MASK_AS_SHORTHAND
>         MOZ_ASSERT(aTable == nsStyleImageLayers::kMaskLayerTable);
>+#endif

>+#ifdef MOZ_ENABLE_MASK_AS_SHORTHAND
>       MOZ_ASSERT(aTable == nsStyleImageLayers::kMaskLayerTable);
>+#endif

These should have an #else with a MOZ_ASSERT_UNREACHABLE() in it.

>diff --git a/layout/style/nsCSSPropAliasList.h b/layout/style/nsCSSPropAliasList.h
>-
>+#ifdef MOZ_ENABLE_MASK_AS_SHORTHAND
> CSS_PROP_ALIAS(-webkit-mask,

Can't you leave -webkit-mask and ifdef just the others?

>diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp
>+#else
>+  // mask: none | <url>
>+  const nsCSSValue* maskValue = aRuleData->ValueForMask();
>+  if (eCSSUnit_URL == maskValue->GetUnit()) {
>+    svgReset->mMask.mLayers[0].mSourceURI = maskValue->GetURLValue();
>+    rebuild = true;
>+  } else if (eCSSUnit_None == maskValue->GetUnit() ||
>+             eCSSUnit_Initial == maskValue->GetUnit() ||
>+             eCSSUnit_Unset == maskValue->GetUnit()) {
>+    svgReset->mMask.mLayers[0].mSourceURI = nullptr;
>+    rebuild = true;
>+  } else if (eCSSUnit_Inherit == maskValue->GetUnit()) {
>+    conditions.SetUncacheable();
>+    svgReset->mMask.mLayers[0].mSourceURI =
>+      parentSVGReset->mMask.mLayers[0].mSourceURI;
>+    rebuild = true;
>+  }
>+#endif
> 
>   if (rebuild) {

Instead of setting rebuild in this block, you should just be able to #ifdef out the maxItemCount and rebuild variables, and the entire if (rebuild) block.



With that, I think this looks good, and I think, assuming that tests pass, that it's a lot safer than the other approach.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8722852 [details] [diff] [review]
0004-Bug-1243734-Part-4.-Set-up-gCSSProperties-depends-on.patch

>+function IsSupportMasAsShorthand() {

This should be called SupportsMaskShorthand.

>+  var div = document.createElement("div");
>+  var url = "url(#myurl)";
>+  div.style.setProperty("mask-image", url);
>+  var maskImageValue = div.style.getPropertyValue("mask-image", "");
>+  return (maskImageValue == url);

Instead of this, you should be able to just do:

  return "maskImage" in document.documentElement.style;



I didn't review the rest in detail yet since you didn't request review, but I'm assuming you're just moving chunks of code appropriately.

Comment 38

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719889 [details]
MozReview Request: Bug 1243734 - Part 3. Set up gCSSProperties depends on mask-as-shorthand; r=dbaron

https://reviewboard.mozilla.org/r/35163/#review33047

This should no longer be needed with the new approach.

Comment 39

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719890 [details]
MozReview Request: Bug 1243734 - Part 4. Set mask-mode reftest as failure before enable mask-as-shorthand; r=dbaron

https://reviewboard.mozilla.org/r/35165/#review33049

I *think* this should no longer be needed with the new approach.

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8720389 [details]
MozReview Request: Bug 1243734 - Part 5. Convert mask-image to mask; r=dbaron

https://reviewboard.mozilla.org/r/35283/#review33053

This will need some sort of replacement with the new approach, but it won't look like this patch.

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8720726 [details]
MozReview Request: Bug 1243734: Part 6. Bring mask-as-shorthand-pref into mochitest.

https://reviewboard.mozilla.org/r/35429/#review33055

Seems like I've already looked at the new version of this patch, so this one isn't needed anymore.

Comment 42

[u459114]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35159/diff/3-4/

Comment 43

[u459114]

Comment on attachment 8719888 [details]
MozReview Request: Bug 1243734 - Part 2. Use MOZ_ENABLE_MASK_AS_SHORTHAND to define the type of mask property; r=dbaron

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35161/diff/3-4/

Comment 44

[u459114]

Comment on attachment 8719889 [details]
MozReview Request: Bug 1243734 - Part 3. Set up gCSSProperties depends on mask-as-shorthand; r=dbaron

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35163/diff/3-4/

Comment 45

[u459114]

Comment on attachment 8719890 [details]
MozReview Request: Bug 1243734 - Part 4. Set mask-mode reftest as failure before enable mask-as-shorthand; r=dbaron

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35165/diff/3-4/

Comment 46

[u459114]

Comment on attachment 8720389 [details]
MozReview Request: Bug 1243734 - Part 5. Convert mask-image to mask; r=dbaron

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35283/diff/2-3/

Comment 47

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

The contents of this attachment are no longer related to the thing I reviewed.  You should ask a build peer for review on this patch.

Comment 48

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://reviewboard.mozilla.org/r/35159/#review33299

Comment 49

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719888 [details]
MozReview Request: Bug 1243734 - Part 2. Use MOZ_ENABLE_MASK_AS_SHORTHAND to define the type of mask property; r=dbaron

https://reviewboard.mozilla.org/r/35161/#review33301

::: layout/style/Declaration.cpp:357
(Diff revision 4)
> +        MOZ_ASSERT_UNREACHABLE("Should never get here when disable mask-as-shorthand");

..."when mask-as-shorthand is disabled"

::: layout/style/Declaration.cpp:380
(Diff revision 4)
> +      MOZ_ASSERT_UNREACHABLE("Should never get here when disable mask-as-shorthand");

same here

Comment 50

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719889 [details]
MozReview Request: Bug 1243734 - Part 3. Set up gCSSProperties depends on mask-as-shorthand; r=dbaron

https://reviewboard.mozilla.org/r/35163/#review33309

I notice that you don't test -webkit-mask here.  However, on second thoughts, I think I was probably wrong in asking you to leave -webkit-mask enabled, since I think the main Web-compatibility use case for -webkit-mask is for the new syntax and not the old syntax.  So I think you should probably leave this patch as-is, but change the previous patch to hide -webkit-mask inside the #ifdef again.  However, you should probably double-check that logic with dholbert first.

Comment 51

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8719890 [details]
MozReview Request: Bug 1243734 - Part 4. Set mask-mode reftest as failure before enable mask-as-shorthand; r=dbaron

https://reviewboard.mozilla.org/r/35165/#review33311

Comment 52

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8720389 [details]
MozReview Request: Bug 1243734 - Part 5. Convert mask-image to mask; r=dbaron

https://reviewboard.mozilla.org/r/35283/#review33313

The commit message should probably say "Convert mask-image to mask" instead of "Remove mask-image used"

Comment 58

[u459114]

Hi Ted,
Since Part 1 changed old-configure.in, please help to review.

Comment 59

[u459114]

(In reply to David Baron [:dbaron] ⌚️UTC-8 from comment #35)
> Comment on attachment 8722849 [details] [diff] [review]
> 0001-Bug-1243734-Part-1.-Add-MOZ_ENABLE_MASK_AS_SHORTHAND.patch
> 
> I don't think you need or want a configure option; this is something that we
> should decide about.  You also don't need an AC_SUBST(), since all you use
> are #ifdefs in C++ files.  So you should just add a commented-out top-level
> AC_DEFINE() line somewhere to indicate that uncommenting it would enable the
> feature.
Actually, AC_SUBST() is needed since we need to to test MOZ_ENABLE_MASK_AS_SHORTHAND in layout/style/test/moz.build:
if CONFIG['MOZ_ENABLE_MASK_AS_SHORTHAND']:
   HOST_DEFINES['MOZ_ENABLE_MASK_AS_SHORTHAND'] = True
   DEFINES['MOZ_ENABLE_MASK_AS_SHORTHAND'] = True

Comment 65

[u459114]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99c71eb074cfacb8fbbbc51c6f7c52d2efe5f464
Bug 1243734 - Part 1. Use MOZ_ENABLE_MASK_AS_SHORTHAND to define the type of mask property; r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca1c774bd78b71e69af4e8804edc1a80ac3cd5c5
Bug 1243734 - Part 2. Set up gCSSProperties depends on mask-as-shorthand; r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/a66ae52f9dc9ddd13baf60f8b94c47ca4247fa5e
Bug 1243734 - Part 3. Set mask-mode reftest as failure before enable mask-as-shorthand; r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd725321463610780d209963e9a6999d52faff0e
Bug 1243734 - Part 4. Convert mask-image to mask; r=dbaron

Comment 66

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/99c71eb074cf
https://hg.mozilla.org/mozilla-central/rev/ca1c774bd78b
https://hg.mozilla.org/mozilla-central/rev/a66ae52f9dc9
https://hg.mozilla.org/mozilla-central/rev/bd7253214636

Comment 67

[(not currently active) Ted Mielczarek]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

https://reviewboard.mozilla.org/r/35159/#review34383

::: old-configure.in:9008
(Diff revision 6)
> +  AC_DEFINE(MOZ_ENABLE_MASK_AS_SHORTHAND)

You don't need both the AC_DEFINE and the DEFINES in the moz.build below. If you only need this defined in that one directory then just use the moz.build version below. If you need this defined globally then you can use AC_DEFINE.

Comment 68

[Sebastian Zartner [:sebo]]

I am wondering why this was implemented as a compiler flag instead of a preference. Or will this preference still be implemented?

Sebastian

Comment 69

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's implemented as a compiler flag because it's not practical to implement as a preference, since we need to continue supporting the old 'mask' syntax when the new syntax is disabled.

Comment 70

[Sebastian Zartner [:sebo]]

Well, I assume it could have been implemented as a boolean preference, where false = old 'mask' syntax and true = new 'mask' syntax plus longhands. With the compiler flag in place authors are not able to try out the new features (if they don't compile the code by themselves).

Sebastian

Comment 71

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Not easily, and not without serious risk of leaving one or the other options quite broken.  See comments above (esp. comment 22 to 24).

Comment 72

[u459114]

https://reviewboard.mozilla.org/r/35159/#review34383

> You don't need both the AC_DEFINE and the DEFINES in the moz.build below. If you only need this defined in that one directory then just use the moz.build version below. If you need this defined globally then you can use AC_DEFINE.

I don't need DEFINES in layout/style/test/moz.build, but I do need HOST_DEFINE in that file. 

Without "HOST_DEFINES['MOZ_ENABLE_MASK_AS_SHORTHAND'] = True" in layout/style/test/moz.build, 
"./mach build -v" got:
/usr/bin/clang++ -o host_ListCSSProperties.o -c -std=gnu++0x -MD -MP -MF .deps/host_ListCSSProperties.o.pp -DDEBUG=1 -DTRACING=1  -I....
Miss -DMOZ_ENABLE_MASK_AS_SHORTHAND and make test case failed.

With HOST_DEFINES
"./mach build -v" got:
/usr/bin/clang++ -o host_ListCSSProperties.o -c -std=gnu++0x -MD -MP -MF .deps/host_ListCSSProperties.o.pp -DDEBUG=1 -DTRACING=1 -DMOZ_ENABLE_MASK_AS_SHORTHAND  -I...
Which is what I need.

I actually have no idea of what is HOST_DEFINES. Please enlighten me if you don't mind...

Comment 73

[u459114]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35159/diff/6-7/

Comment 74

[(not currently active) Ted Mielczarek]

Comment on attachment 8719887 [details]
MozReview Request: Bug 1243734 - Part 1. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag;

https://reviewboard.mozilla.org/r/35159/#review34661

Comment 75

[(not currently active) Ted Mielczarek]

https://reviewboard.mozilla.org/r/35159/#review34383

> I don't need DEFINES in layout/style/test/moz.build, but I do need HOST_DEFINE in that file. 
> 
> Without "HOST_DEFINES['MOZ_ENABLE_MASK_AS_SHORTHAND'] = True" in layout/style/test/moz.build, 
> "./mach build -v" got:
> /usr/bin/clang++ -o host_ListCSSProperties.o -c -std=gnu++0x -MD -MP -MF .deps/host_ListCSSProperties.o.pp -DDEBUG=1 -DTRACING=1  -I....
> Miss -DMOZ_ENABLE_MASK_AS_SHORTHAND and make test case failed.
> 
> With HOST_DEFINES
> "./mach build -v" got:
> /usr/bin/clang++ -o host_ListCSSProperties.o -c -std=gnu++0x -MD -MP -MF .deps/host_ListCSSProperties.o.pp -DDEBUG=1 -DTRACING=1 -DMOZ_ENABLE_MASK_AS_SHORTHAND  -I...
> Which is what I need.
> 
> I actually have no idea of what is HOST_DEFINES. Please enlighten me if you don't mind...

HOST_DEFINES is just the defines we use for compiling host binaries (binaries that run on the system doing the build, our terminology doesn't match modern autoconf here). We build ListCSSProperties as a host binary, so it needs this define.

Comment 76

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked, will be happy to uplift to Aurora47 once the fix is ready and baked enough on Nightly.

Comment 77

[u459114]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7fd5da36595139efc6d103249b36a5338d0efb9
Bug 1243734 - Part 5. Add MOZ_ENABLE_MASK_AS_SHORTHAND compile flag; r=ted r=dbaron

Comment 78

[Carsten Book [:Tomcat]]

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

Comment 79

[u459114]

(In reply to Ritu Kothari (:ritu) from comment #76)
> Tracked, will be happy to uplift to Aurora47 once the fix is ready and baked
> enough on Nightly.

Thanks, but no need. I had pushed part 2 ~ part 4 into gecko 47 before 3/7, please see comment 66, which is all we need to disable image-mask. Part 1 can be stayed in gecko 48.