Closed Bug 1341667 Opened 7 years ago Closed 7 years ago

stylo: Figure out why all the W3C masking reftests fail

Categories

(Core :: CSS Parsing and Computation, defect, P1)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: bzbarsky, Assigned: u459114)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Bug 1341667 - Part 1. Keep url in nsStyleImage for local-fragment url.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1341667 - Part 2. Enable css-masking reftests in stylo.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
Pretty much everything in layout/reftests/w3c-css/submitted/masking/reftest-stylo.list is marked as failing.
Priority: -- → P2
Over to CJ for investigation.
Assignee: nobody → cku
Priority: P2 → P1
All nsStyleSVGReset attributes at gecko side are empty, so no nsDisplayMask been created.
I guess something wrong in gecko.mako.rs(I did not see style_struct_name="SVGReset" statement in that rs file).
stylo has the opposite naming convention, so SVG->SVGInherited and SVGReset->SVG.

So you want http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/servo/components/style/properties/gecko.mako.rs#3001
IC, thanks, bholley.

http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/servo/components/style/properties/gecko.mako.rs#2288

There are two types of mask-image, image mask and SVG mask
#2288 does not setup image mask correctly and #2288 does not handle SVG mask at all. Working on the patch.
Depends on: 1352096
Attachment #8853080 - Flags: review?(cam)
Attachment #8856365 - Flags: review?(cam)
Attachment #8853079 - Flags: review?(cam)
Comment on attachment 8853079 [details]
Bug 1341667 - Part 1. Keep url in nsStyleImage for local-fragment url.

https://reviewboard.mozilla.org/r/125166/#review131232
Attachment #8853079 - Flags: review?(cam) → review+
Comment on attachment 8853080 [details]
Bug 1341667 - Part 2. (Stylo) Correct computed value of mask-image.

https://reviewboard.mozilla.org/r/125168/#review131236

r=me with this.

::: servo/components/style/properties/longhand/svg.mako.rs:264
(Diff revision 3)
>      pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
>          if input.try(|input| input.expect_ident_matching("none")).is_ok() {
>              Ok(SpecifiedValue::None)
>          } else {
>              let image = try!(Image::parse(context, input));
>              match image {
>                  Image::Url(url_value) => {
> -                    if url_value.is_fragment() {
> -                        Ok(SpecifiedValue::Url(url_value))
> -                    } else {
> -                        Ok(SpecifiedValue::Image(Image::Url(url_value)))
> +                    Ok(SpecifiedValue::Image(Image::Url(url_value)))
> -                    }
> +                }
> -                }
>                  image => Ok(SpecifiedValue::Image(image))
>              }
>          }

Does this change mean we never set a SpecifiedValue::Url value any more, and we also never use computed_value::T::Url?  If so, remove these types from the enums.
Attachment #8853080 - Flags: review?(cam) → review+
Comment on attachment 8856365 [details]
Bug 1341667 - Part 3. Enable css-masking reftests in stylo.

https://reviewboard.mozilla.org/r/128290/#review131238
Attachment #8856365 - Flags: review?(cam) → review+
Attachment #8853080 - Attachment is obsolete: true
Attachment #8856365 - Attachment is obsolete: true
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40afcc9cae69
Part 1. Keep url in nsStyleImage for local-fragment url. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff7c4ec7e0db
Part 2. Enable css-masking reftests in stylo. r=heycam
backed this out for causing stylo tests busted in m-i, and this should be landed in autoland

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=79bef3ce0ad383522d0098cbb00ce1bee137a573
Flags: needinfo?(cku)
Comment on attachment 8857332 [details]
Bug 1341667 - Part 2. Enable css-masking reftests in stylo.

https://reviewboard.mozilla.org/r/129312/#review131832
Attachment #8857332 - Flags: review?(cam) → review+
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/309d2b0b0f4e
Backed out changeset ff7c4ec7e0db for stylo tests bustage and the patches should be landed to autoland
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1b487ab8d26
Part 1. Keep url in nsStyleImage for local-fragment url. r=heycam
https://hg.mozilla.org/integration/autoland/rev/6be0200a4446
Part 2. Enable css-masking reftests in stylo. r=heycam
Flags: needinfo?(cku)
https://hg.mozilla.org/mozilla-central/rev/f1b487ab8d26
https://hg.mozilla.org/mozilla-central/rev/6be0200a4446
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: