Boris Chiou [:boris] Assignee
	Description • 6 years ago

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

	Brian Birtles (:birtles)
	Comment 1 • 6 years ago

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?

	Boris Chiou [:boris] Assignee
	Comment 2 • 6 years ago

(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?

	Xidorn Quan [:xidorn] UTC+10
	Comment 3 • 6 years ago

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.

	Boris Chiou [:boris] Assignee
	Comment 4 • 6 years ago

(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.

	Sean Voisen (:svoisen)
	Comment 5 • 6 years ago

(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.

	Boris Chiou [:boris] Assignee
	Comment 6 • 6 years ago

(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.

	Boris Chiou [:boris] Assignee
	Comment 7 • 6 years ago

Add a preference, layout.css.clip-path-path.enabled, for |clip-path:path()|.

	Boris Chiou [:boris] Assignee
	Comment 8 • 6 years ago

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

	Boris Chiou [:boris] Assignee
	Comment 9 • 6 years ago

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

	Xidorn Quan [:xidorn] UTC+10
	Comment 10 • 6 years ago

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

Xidorn Quan [:xidorn] UTC+10 has approved the revision.

	Xidorn Quan [:xidorn] UTC+10
	Comment 11 • 6 years ago

Comment on attachment 9006343 [details]
Bug 1487838 - Move clip-path path reftest into wpt.

Xidorn Quan [:xidorn] UTC+10 has approved the revision.

	Pulsebot
	Comment 13 • 6 years ago

Pushed by boris.chiou@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/684b7f485239
Add a pref for |clip-path:path()|. r=xidorn

	Pulsebot
	Comment 14 • 6 years ago

Pushed by boris.chiou@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5f85f154f4e8
Move clip-path path reftest into wpt. r=xidorn

	Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)
	Comment 15 • 6 years ago

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12842 for changes under testing/web-platform/tests

	Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)
	Comment 16 • 6 years ago

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

	Dorel Luca [:dluca]
	Comment 17 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/684b7f485239
https://hg.mozilla.org/mozilla-central/rev/5f85f154f4e8

	Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)
	Comment 18 • 6 years ago

Upstream PR merged

	Chris Mills [:cmills]
	Comment 19 • 6 years ago

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?

	Boris Chiou [:boris] Assignee
	Comment 20 • 6 years ago

(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.