Closed
Bug 1265343
Opened 8 years ago
Closed 6 years ago
stylo: Implement 'shape-image-threshold'
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: sebo, Assigned: TYLin)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-complete, DevAdvocacy)
Attachments
(3 files, 1 obsolete file)
The shape-image-threshold property should be implemented as defined in https://drafts.csswg.org/css-shapes/#shape-margin-property. Sebastian
Reporter | ||
Comment 1•8 years ago
|
||
Sorry, wrong link. Here's the correct one: https://drafts.csswg.org/css-shapes/#shape-image-threshold-property Sebastian
Reporter | ||
Updated•8 years ago
|
Depends on: shape-outside
Reporter | ||
Updated•8 years ago
|
Keywords: dev-doc-needed
Updated•8 years ago
|
Keywords: DevAdvocacy
Updated•8 years ago
|
Whiteboard: [DevRel:P1]
Updated•8 years ago
|
Flags: platform-rel?
Updated•8 years ago
|
platform-rel: --- → ?
Updated•8 years ago
|
platform-rel: ? → ---
Updated•7 years ago
|
Whiteboard: [DevRel:P1]
I notice this was marked as depending on bug 435426. I suppose this dependency makes some sense given the example in https://drafts.csswg.org/css-shapes/#shapes-from-image -- however, if other browsers have shipped shape-image-threshold without shipping 'attr(src url)', I don't think we should wait either. (On the other hand, if other browsers haven't, but have instead shipped shape-outside without shipping shape-image-threshold, then this shouldn't block shipping shape-outside.)
Reporter | ||
Comment 3•7 years ago
|
||
I tested Chrome and Opera 47 right now and both support shape-image-threshold without supporting 'attr(src url)'. According to MDN[1] shape-image-threshold is also supported by Safari, though none of the browsers currently supports 'attr()' on other properties than 'content', and also only the one-value syntax[2]. Disregarding the notes above, I believe Mozilla's decision should not depend on the decision of other browsers in this case. I think this property is already useful without the support of 'attr(src url)' for shape-outside. AND, Astley already removed the dependency from shape-outside in bug 1098939 comment 6, so it's just logical to remove it here, too. Having said that, it would still be great if 'attr(src url)' (for other properties than 'content') would be pushed, as it opens up a lot of ways for interaction between HTML and CSS. Sebastian [1] https://developer.mozilla.org/en-US/docs/Web/CSS/shape-image-threshold [2] https://developer.mozilla.org/en-US/docs/Web/CSS/attr
No longer depends on: 435426
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Blocks: shape-outside
No longer depends on: shape-outside
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6710706c0bfdf91bacc55987f0c70b0751a3b39e
Assignee | ||
Comment 8•6 years ago
|
||
Add 'stylo' keyword to the bug title. Because 'shape-image-threshold' property itself doesn't render anything, it's considered done after adding support to the style system. The rendering part of shape-outside: <image> will be done in bug 1404222.
Summary: Implement 'shape-image-threshold' → stylo: Implement 'shape-image-threshold'
Assignee | ||
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8932687 [details] Bug 1265343 Part 2 - Fix tests after adding 'shape-image-threshold' to style system. https://reviewboard.mozilla.org/r/203554/#review209226 ::: testing/web-platform/meta/css/css-shapes/shape-outside/values/shape-image-threshold-003.html.ini:3 (Diff revision 1) > [shape-image-threshold is not inherited and defaults to 0] > expected: FAIL I thought this one should pass, but I got `assert_equals: expected (object) null but got (string) ""`. Shouldn `getPropertyValue()` return `null` if it not set? https://treeherder.mozilla.org/#/jobs?repo=try&author=tlin@mozilla.com&selectedJob=148196932
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8932686 [details] Bug 1265343 Part 1 - Add shape-image-threshold to style system. https://reviewboard.mozilla.org/r/203552/#review209664 ::: layout/style/nsStyleStruct.cpp:3792 (Diff revision 1) > if (aNewData.mFloat != StyleFloat::None) { > // If we are floating, and our shape-outside property changes, our > // descendants are not impacted, but our ancestor and siblings are. This and the other comment below should be updated to mention shape-image-threshold too. ::: layout/style/nsStyleStruct.cpp:3806 (Diff revision 1) > // shape-outside changed, but we don't need to reflow because we're not > // floating. Update this comment to mention shape-image-threadhsold too
Attachment #8932686 -
Flags: review?(cam) → review+
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8932688 [details] style: Support 'shape-image-threshold' property https://reviewboard.mozilla.org/r/203556/#review209668 ::: servo/components/style/properties/gecko.mako.rs:3102 (Diff revision 1) > - shape-outside contain touch-action""" %> > + shape-image-threshold shape-outside > + contain touch-action""" %> I think we can remove all of these changes to gecko.mako.rs, since the property is defined with the Opacity type, and the glue functions will get generated automatically for that type. (At least I think it should -- that's how it works for the 'opacity' property.)
Attachment #8932688 -
Flags: review?(cam) → review+
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8932687 [details] Bug 1265343 Part 2 - Fix tests after adding 'shape-image-threshold' to style system. https://reviewboard.mozilla.org/r/203554/#review209672
Attachment #8932687 -
Flags: review?(cam) → review+
Assignee | ||
Comment 13•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8932688 [details] style: Support 'shape-image-threshold' property https://reviewboard.mozilla.org/r/203556/#review209668 > I think we can remove all of these changes to gecko.mako.rs, since the property is defined with the Opacity type, and the glue functions will get generated automatically for that type. (At least I think it should -- that's how it works for the 'opacity' property.) Neat! All the changes to gecko.mako.rs are not needed.
Assignee | ||
Comment 14•6 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8932688 -
Attachment is obsolete: true
Assignee | ||
Comment 17•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8932687 [details] Bug 1265343 Part 2 - Fix tests after adding 'shape-image-threshold' to style system. https://reviewboard.mozilla.org/r/203554/#review209226 > I thought this one should pass, but I got `assert_equals: expected (object) null but got (string) ""`. > > Shouldn `getPropertyValue()` return `null` if it not set? > > https://treeherder.mozilla.org/#/jobs?repo=try&author=tlin@mozilla.com&selectedJob=148196932 This test is fixed by upstream. https://github.com/w3c/web-platform-tests/pull/8517
Comment 18•6 years ago
|
||
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/929e3111144c Part 1 - Add shape-image-threshold to style system. r=heycam https://hg.mozilla.org/integration/autoland/rev/853a847067ad Part 2 - Fix tests after adding 'shape-image-threshold' to style system. r=heycam
Comment 19•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/929e3111144c https://hg.mozilla.org/mozilla-central/rev/853a847067ad
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•6 years ago
|
status-firefox57:
--- → wontfix
status-firefox58:
--- → wontfix
Comment 20•6 years ago
|
||
I was going to write a page about this, but then I found out that it had already been done: https://developer.mozilla.org/en-US/docs/Web/CSS/shape-image-threshold So I've just added a note to the Fx59 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/59#CSS Let me know if that is OK. Thanks!
Flags: needinfo?(aethanyc)
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 21•6 years ago
|
||
We shouldn't add this to the 59 release note because this property is still behind the pref "layout.css.shape-outside.enabled".
Flags: needinfo?(aethanyc) → needinfo?(cmills)
Comment 22•6 years ago
|
||
Hi Chris, Yeah, there are many parts and pieces for CSS Shapes level 1, including: shape-outside, shape-margin, shape-image-threshold (just off the top of my head). They will all ship together, not one at a time, since they are needed together. Work is still underway. There was a hope that perhaps CSS Shapes would make it for Firefox 60, but it definitely won't happen for 59. And it might not make 60 — instead sliding to 61 or whenever it's ready. It would be great for all the CSS Shapes docs to be all shiny and happy. Also — for the docs for the Shape Path Editor to be ready. We'll be shipping the Shape Path Editor at the same time.
Comment 23•6 years ago
|
||
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) (Away, needinfo me if you need anything) from comment #21) > We shouldn't add this to the 59 release note because this property is still > behind the pref "layout.css.shape-outside.enabled". OK, thanks. I've removed the note from the rel notes, and: * Added an entry in our experimental features page: https://developer.mozilla.org/en-US/Firefox/Experimental_features * Updated the browser support info to include the flag information: https://github.com/mdn/browser-compat-data/pull/899
Flags: needinfo?(cmills)
Comment 24•6 years ago
|
||
(In reply to Jen Simmons [:jensimmons] from comment #22) > Hi Chris, > Yeah, there are many parts and pieces for CSS Shapes level 1, including: > shape-outside, shape-margin, shape-image-threshold (just off the top of my > head). They will all ship together, not one at a time, since they are needed > together. Work is still underway. There was a hope that perhaps CSS Shapes > would make it for Firefox 60, but it definitely won't happen for 59. And it > might not make 60 — instead sliding to 61 or whenever it's ready. It would > be great for all the CSS Shapes docs to be all shiny and happy. Also — for > the docs for the Shape Path Editor to be ready. We'll be shipping the Shape > Path Editor at the same time. Cool, thanks for all the info Jen; I've contacted you over mail for followup.
You need to log in
before you can comment on or make changes to this bug.
Description
•