Closed Bug 1487838 Opened 6 years ago Closed 6 years ago

Add a pref for |clip-path:path()|

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: boris, Assigned: boris)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

We implement |clip-path:path()| in Bug 1246764. However, it is chrome-only for now. File this bug for tracking its status. If we decide to make it public, we could update it here [1].

[1] https://searchfox.org/mozilla-central/rev/05d91d3e02a0780f44599371005591d7988e2809/servo/components/style/values/specified/basic_shape.rs#58-66
Priority: -- → P5
Is there any reason we don't want to ship it? Are there outstanding spec issues? Insufficient WPT coverage? Other features we want to ship at the same time?
Flags: needinfo?(boris.chiou)
(In reply to Brian Birtles (:birtles) from comment #1)
> Is there any reason we don't want to ship it? Are there outstanding spec
> issues? Insufficient WPT coverage? Other features we want to ship at the
> same time?

We will ship this eventually, I think. I'm not sure if the spec is stable or not, and it seems |clip-path:path()| is used by the front-end only for now, so I make it as chrome-only value. I'm happy to ship this ASAP, so it would be easier to write the tests.

Actually I'm not sure what should I do (i.e. ship it or use a pref or keep using chrome-only) for this. The WPT coverage of Path function is sufficient, and we only need to add some for clip-path:path() to make sure it works.

Looks like no other features need to be shipped at the same time.


Sean has a meeting with devtool guys, and maybe he has some plans for this?
Flags: needinfo?(boris.chiou) → needinfo?(svoisen)
I would suggest we put it behind a pref regardless the shipping plan, so that we can have tests for it, devtools team can play with it while developing any tool for it, and other interested people can also try it and provide feedback.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> I would suggest we put it behind a pref regardless the shipping plan, so
> that we can have tests for it, devtools team can play with it while
> developing any tool for it, and other interested people can also try it and
> provide feedback.

Agreed. This makes us easier to test this. I could add a pref (e.g. layout.css.clip-path-path.enabled) for this, and move the tests into wpt.
(In reply to Boris Chiou [:boris] from comment #2)

> Sean has a meeting with devtool guys, and maybe he has some plans for this?

Yes, the update is that DevTools will be waiting a bit on this, so we will keep as chrome-only for now. It's in the DevTools backlog to support. What they have said is that they will ensure that the current shape editor tool will at least not break when inspecting a path in this format.

That said, I agree that we should have a pref, so please go ahead and add it and file a separate bug for actually flipping it on. But I'd prefer to flip it on in nightly when the shape editor will support it.
Flags: needinfo?(svoisen)
Priority: P5 → P3
(In reply to Sean Voisen (:svoisen) from comment #5)
> (In reply to Boris Chiou [:boris] from comment #2)
> 
> > Sean has a meeting with devtool guys, and maybe he has some plans for this?
> 
> Yes, the update is that DevTools will be waiting a bit on this, so we will
> keep as chrome-only for now. It's in the DevTools backlog to support. What
> they have said is that they will ensure that the current shape editor tool
> will at least not break when inspecting a path in this format.
> 
> That said, I agree that we should have a pref, so please go ahead and add it
> and file a separate bug for actually flipping it on. But I'd prefer to flip
> it on in nightly when the shape editor will support it.

Cool. Let me add a pref and default off. (Enable it only for test.) Thanks for this info.
Assignee: nobody → boris.chiou
Summary: Make |clip-path:path()| public or before a pref → Add a pref for |clip-path:path()|
Blocks: 1488530
Add a preference, layout.css.clip-path-path.enabled, for |clip-path:path()|.
Move the reftests of path() into wpt. For the rest reftests of clip-path, we
could move them into wpt in a separate bug.

Depends on D4965
Comment on attachment 9006327 [details]
Bug 1487838 - Add a pref for |clip-path:path()|.

Xidorn Quan [:xidorn] UTC+10 has approved the revision.
Attachment #9006327 - Flags: review+
Comment on attachment 9006343 [details]
Bug 1487838 - Move clip-path path reftest into wpt.

Xidorn Quan [:xidorn] UTC+10 has approved the revision.
Attachment #9006343 - Flags: review+
Pushed by boris.chiou@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/684b7f485239
Add a pref for |clip-path:path()|. r=xidorn
Pushed by boris.chiou@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5f85f154f4e8
Move clip-path path reftest into wpt. r=xidorn
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12842 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
https://hg.mozilla.org/mozilla-central/rev/684b7f485239
https://hg.mozilla.org/mozilla-central/rev/5f85f154f4e8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
I added an entry to our Experimental Features page:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Experimental_features#CSS

I think this covers it for now?
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #19)
> I added an entry to our Experimental Features page:
> https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/
> Experimental_features#CSS
> 
> I think this covers it for now?

Yes. That's correct on the MDN. (i.e. we disable this pref on nightly and other versions.) Thanks for this.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: