1272004 - Create test case for fallback logic between mask-mode and mask-type

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

css-masking spec said:
The mask-type property allows the author of the mask element to specify the preferred masking mode. However, the author can override this preference by setting the mask-mode value to something different than auto on the masked content.

Have test cases to verify this logic

[1] https://www.w3.org/TR/css-masking-1/#propdef-mask-type

Comment 1

[Ethan Lin[:ethlin]]

I will work on this.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Add test to check the logic between mask-mode and mask-type.

The test case is for checking the logic between mask-type and mask-mode. So there should be six combinations ([mask-source, alpha, luminance] x [alpha, luminance]).

Comment 3

[Cameron McCormack (:heycam)]

Comment on attachment 8754701 [details] [diff] [review]
Add test to check the logic between mask-mode and mask-type.

Review of attachment 8754701 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/w3c-css/submitted/masking/mask-type.html
@@ +69,5 @@
> +    <div class="alpha-alpha"></div>
> +    <svg xmlns="http://www.w3.org/2000/svg">
> +      <defs>
> +        <mask id="svg-luminance" maskContentUnits="objectBoundingBox"
> +            style="mask-type: luminance">

Since most of the test details are in the <style> element above, I think it would make sense to move the mask-type declaration into the style sheet too.

@@ +71,5 @@
> +      <defs>
> +        <mask id="svg-luminance" maskContentUnits="objectBoundingBox"
> +            style="mask-type: luminance">
> +          <rect x="0" y="0" width="100%" height="100%"
> +              fill="blue" fill-opacity="1"/>

The initial value of fill-opacity is 1 anyway, so no need to include it here (and below).

Comment 4

[Ethan Lin[:ethlin]]

(In reply to Cameron McCormack (:heycam) from comment #3)
> Comment on attachment 8754701 [details] [diff] [review]
> Add test to check the logic between mask-mode and mask-type.
> 
> Review of attachment 8754701 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/reftests/w3c-css/submitted/masking/mask-type.html
> @@ +69,5 @@
> > +    <div class="alpha-alpha"></div>
> > +    <svg xmlns="http://www.w3.org/2000/svg">
> > +      <defs>
> > +        <mask id="svg-luminance" maskContentUnits="objectBoundingBox"
> > +            style="mask-type: luminance">
> 
> Since most of the test details are in the <style> element above, I think it
> would make sense to move the mask-type declaration into the style sheet too.
> 
I'll move it into style block.

> @@ +71,5 @@
> > +      <defs>
> > +        <mask id="svg-luminance" maskContentUnits="objectBoundingBox"
> > +            style="mask-type: luminance">
> > +          <rect x="0" y="0" width="100%" height="100%"
> > +              fill="blue" fill-opacity="1"/>
> 
> The initial value of fill-opacity is 1 anyway, so no need to include it here
> (and below).

Okay, I will remove it.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: Add test to check the logic between mask-mode and mask-type. (carry r+: heycam)

Address :heycam's comments.

Comment 6

[Ethan Lin[:ethlin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=be6e5b24e7fd

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d7c5acc9b1f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7d7c5acc9b1f

Comment 9

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

One issue with the submission to the W3C reftests, from check-for-references.sh:

Missing link for ./masking/mask-type.html

which says that this test isn't going to be recognized as a reftest by the W3C harness due to lack of a link rel="match".


Also, mask-type.html is probably not a sufficiently-unique name for a reftest.  The name should say a little bit more about what's being tested.

Comment 10

[Ethan Lin[:ethlin]]

(In reply to David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) from comment #9)
> One issue with the submission to the W3C reftests, from
> check-for-references.sh:
> 
> Missing link for ./masking/mask-type.html
> 
> which says that this test isn't going to be recognized as a reftest by the
> W3C harness due to lack of a link rel="match".
> 
> 
> Also, mask-type.html is probably not a sufficiently-unique name for a
> reftest.  The name should say a little bit more about what's being tested.

Sorry for that. I will be careful. I opened bug 1276435 to fix it.