Closed Bug 1272004 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: u459114, Assigned: ethlin)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Assignee: nobody → cku
Target Milestone: --- → mozilla49
Summary: Ask test case for fallback logic between mask-mode and mask-type → Create test case for fallback logic between mask-mode and mask-type
I will work on this.
Assignee: cku → ethlin
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]).
Attachment #8754701 - Flags: review?(cam)
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).
Attachment #8754701 - Flags: review?(cam) → review+
(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.
Address :heycam's comments.
Attachment #8754701 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7d7c5acc9b1f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
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.
Flags: needinfo?(ethlin)
Blocks: 1276435
(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.
Flags: needinfo?(ethlin)
You need to log in before you can comment on or make changes to this bug.