u459114 Assignee
	Description • 8 years ago

According to the spec
https://drafts.fxtf.org/css-masking-1/#the-mask-image
Name: 	mask-image
Applies to: 	All elements. In SVG, it applies to container elements excluding the <defs> element and all graphics elements
All mask longhands should be able to apply to svg elements.

We currently only allow SVG-mask apply onto svg element:
http://searchfox.org/mozilla-central/source/layout/svg/nsSVGUtils.cpp#528

Turning it on is not too difficult, test cases needed.

	u459114 Assignee
	Comment 5 • 8 years ago

It may not be a blocker of shipping positioned mask since neither safari or chrome support it.

But it does a blocker of  Bug 1303623 and Bug 1311270. mask-clip:fill-box/stroke-box/view-box takes effect only when applied to an SVG element. We can not even test the solution in 1311270 without fix here.

	Astley Chen (inactive)
	Comment 8 • 8 years ago

Thanks for opening this bug and making CSS masking support on Firefox more complete.

	Markus Stange [:mstange]
	Comment 14 • 8 years ago
mozreview-review

Comment on attachment 8813540 [details]
Bug 1319667 - Part 1. Allow mask longhands apply to SVG elements.

https://reviewboard.mozilla.org/r/94990/#review95302

::: layout/svg/nsSVGIntegrationUtils.cpp:601
(Diff revision 5)
> +    //   we have any item in maskFrame even if all of those items are
> +    //   non-resolvable <mask-sources> or <images>.
> +    //   Set paintResult.transparentBlackMask as true,  the caller should stop
> +    //   painting masked content as if this mask is a transparent black one.
> +    // For a SVG doc:
> +    //   SVG 1.1 say that  if we fail to resolve a mask, we should draw the

there's two spaces between "that" and "if"

	Markus Stange [:mstange]
	Comment 15 • 8 years ago
mozreview-review

Comment on attachment 8813742 [details]
Bug 1319667 - Part 2. Move DrawResult from function parameter of PaintClipMask to the return value.

https://reviewboard.mozilla.org/r/95132/#review95310

	Markus Stange [:mstange]
	Comment 16 • 8 years ago
mozreview-review

Comment on attachment 8813541 [details]
Bug 1319667 - Part 3. Test case for mask longhands on svg element.

https://reviewboard.mozilla.org/r/94992/#review95312

	Pulsebot
	Comment 21 • 8 years ago

Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/464230532ba3
Part 1. Allow mask longhands apply to SVG elements. r=mstange
https://hg.mozilla.org/integration/autoland/rev/3ab240e653d8
Part 2. Move DrawResult from function parameter of PaintClipMask to the return value. r=mstange
https://hg.mozilla.org/integration/autoland/rev/d9b26b438895
Part 3. Test case for mask longhands on svg element. r=mstange

	Carsten Book [:Tomcat]
	Comment 22 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/464230532ba3
https://hg.mozilla.org/mozilla-central/rev/3ab240e653d8
https://hg.mozilla.org/mozilla-central/rev/d9b26b438895

	Pulsebot
	Comment 23 • 8 years ago

Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c63c761e089c
(follow-up) Remove incorrect assertion. r=me

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 24 • 8 years ago
bugherder

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

	Andrei Vaida [:avaida]
	Comment 25 • 7 years ago

C.J. Ku, does this feature have automated coverage? Would it benefit from manual testing?

	u459114 Assignee
	Comment 26 • 7 years ago

S(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #25)
> C.J. Ku, does this feature have automated coverage? Would it benefit from
> manual testing?
There is a ref-test case in Part 3. May I know which kind of test that you asking?

	Andrei Vaida [:avaida]
	Comment 27 • 7 years ago

(In reply to C.J. Ku[:cjku](UTC+8) from comment #26)
> S(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #25)
> > C.J. Ku, does this feature have automated coverage? Would it benefit from
> > manual testing?
> There is a ref-test case in Part 3. May I know which kind of test that you
> asking?

Sure, I was just wondering if manual QA should be tracking this bug on Fx53. But since it has automated coverage, in the form of reftests, I think it's not necessary (or at least not as necessary as for other bug fixes that are currently riding Fx53).

Thank you for following up!