1383650 - Implement SVG geometry properties in CSS

Status: RESOLVED FIXED

(Core :: SVG, enhancement, P3)

Description

[u459114]

According to SVG 2.0 spec[1], the CSS width and height properties are used for sizing some SVG elements. We do not support this yet

(It's not clear to me why x/y are also available in css properties on chrome.) 

[1] https://svgwg.org/svg2-draft/single-page.html#styling-StylingUsingCSS

Comment 1

[Cameron McCormack (:heycam)]

'x' and 'y' properties are defined here: https://svgwg.org/svg2-draft/geometry.html

Comment 2

[u459114]

(In reply to Cameron McCormack (:heycam) from comment #1)
> 'x' and 'y' properties are defined here:
> https://svgwg.org/svg2-draft/geometry.html

but, it doesn't say x/y can be defined in css properties? I think they can be set in presentation attributes only

Comment 3

[Cameron McCormack (:heycam)]

The definitions on that page are meant to be CSS property definitions.  I think presentation attributes by definition mean attributes that set a corresponding CSS property.

(If you feel it's a bit odd to have properties named 'x' and 'y', it's true, but the SVGWG spent a long time going back and forth on the best way to allow the main geometric aspects of SVG elements to be styled using CSS, and this was the approach that we finally settled on and got agreement from the CSSWG on.)

Comment 4

[u459114]

Thanks!

That means we should support the following CSS propeties in svg:
1. The cx/cy property
2. The r/rx/ry property
3. The x/y property
4. Sizing properties: width and height

Comment 5

[Patrick Dark]

(In reply to C.J. Ku[:cjku](UTC+8) from comment #4)
> Thanks!
> 
> That means we should support the following CSS propeties in svg:
> 1. The cx/cy property
> 2. The r/rx/ry property
> 3. The x/y property
> 4. Sizing properties: width and height

There’s at least one property missing from that list: the |d| property. See
https://svgwg.org/svg2-draft/paths.html#TheDProperty.

Comment 6

[Patrick Dark]

Microsoft Edge Development Issue: https://developer.microsoft.com/microsoft-edge/platform/issues/11543103/
Monorail Issue: https://bugs.chromium.org/p/chromium/issues/detail?id=652822

I don’t see a generic issue for SVG geometry properties on Monorail. (The linked issue only applies to the |d| property.)

Comment 8

[Thomas Wisniewski [:twisniewski]]

It seems that there's enough uptake for this CSS to cause a live webcompat issue affecting www.publicmobile.ca, which only specifies cx, cy, and r for their little SVG badge circles for 3G etc in CSS, leaving those badges invisible in Gecko (as reported in https://webcompat.com/issues/26401).

Comment 11

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 12

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 13

[violet.bugreport]

Attachment: WIP-patch.patch

Implementing CSS property for x, I think there are these steps:

1. Add an entry at servo/components/style/properties/longhands/inherited_svg.mako.rs, and layout/style/nsStyleStruct.h

2. Map SVG attribute to CSS by overriding IsAttributeMapped(), so that a value set in SVG will be known by CSS part.

3. Override DidSetComputedStyle for the frame, so that we are aware of the CSS change.

The cursory WIP patch seems to be working fine on my local machine, and the CSS transition stuff works just automatically...

Emilio, do you think there is anything I'm missing on the CSS part? In particular, I'd like to know:

1. Is nsComputedDOMStyle::GetComputedStyleNoFlush very cheap? Is it better to store the CSS value in the SVG element then update them when DidSetComputedStyle?

2. I have to add X_CONTEXT constant in nsStyleStruct.h, otherwise it doesn't compile. But it's meaningless for x... Do we need to update some Rust code so that we don't need to declare this constant?

Thanks!

Comment 14

[Emilio Cobos Álvarez (:emilio)]

(In reply to violet.bugreport from comment #13)

1. Add an entry at servo/components/style/properties/longhands/inherited_svg.mako.rs, and layout/style/nsStyleStruct.h

2. Map SVG attribute to CSS by overriding IsAttributeMapped(), so that a value set in SVG will be known by CSS part.

3. Override DidSetComputedStyle for the frame, so that we are aware of the CSS change.

You need to change the relevant style struct's CalcDifference function to at least return NeutralChange if the property has changed. If you add an entry to property_database.js, you get some tests that test the implications of this for free.

You can just return InvalidateRenderingObservers instead of NeutralChange + overriding DidSetComputedStyle. That means that InvalidateRenderingObservers will be unnecessarily called for some kind of frames where it's not needed / the property does not apply, but that's a more general problem, see bug 1474505.

1. Is nsComputedDOMStyle::GetComputedStyleNoFlush very cheap? Is it better to store the CSS value in the SVG element then update them when DidSetComputedStyle?

It's relatively cheap, but it's not guaranteed to return non-null (if the element is not styled yet, or is in a display: none subtree. In this case, seems it'd be better to use GetPrimaryFrame()->Style(), which is much cheaper. Same caveat, element may have no frame if not rendered yet. But CSS transitions don't apply to non-rendered elements anyway, so it seems we may be able to assert there is a frame?

2. I have to add X_CONTEXT constant in nsStyleStruct.h, otherwise it doesn't compile. But it's meaningless for x... Do we need to update some Rust code so that we don't need to declare this constant?

That's because you're using SVGLength, which parses both a length and context-value. If the x property doesn't support context-value (which it doesn't), it shouldn't use SVGLength, and should use a plain LengthPercentage. You need to parse with quirks always enabled (AllowQuirks::Always). Looks like this would be the first property doing that, so need some changes.

You can do it in two ways, I slightly prefer the second since we already have all the parse_quirky stuff in place and so it would apply to any of the relevant types:

• Add a pub fn parse_always_quirky(context, input) to LengthPercentage in values/specified/length.rs, and use parse_function="parse_always_quirky" in the property entry.

• Tweak allow_quirks to get a tri-state ["Yes", "No", "Always"] instead of a boolean, and change the generated code to allow emitting AllowQuirks::Always here and in similar call-sites in that file.

Hope it helps, if you get stuck I'm happy to do those style system bits, just lmk :)

Thank you for looking into this!

Comment 15

[Robert Longson [:longsonr]]

If it helps we already map the width and height attributes of outer <svg> elements to style. This was originally implemented in bug 668163 and bug 764851

Comment 16

[violet.bugreport]

Attachment: Bug 1383650 - Support AllowQuirks::Always option in helpers.mako.rs

Comment 17

[violet.bugreport]

Attachment: Bug 1383650 - Add SVG geometry property support in CSS

This patch adds SVG geometry properties to CSS, it doesn't deal with
how SVG handles them.

Comment 19

[violet.bugreport]

(In reply to violet.bugreport from comment #13)

Implementing CSS property for x, I think there are these steps:

1. Add an entry at servo/components/style/properties/longhands/inherited_svg.mako.rs, and layout/style/nsStyleStruct.h

2. Map SVG attribute to CSS by overriding IsAttributeMapped(), so that a value set in SVG will be known by CSS part.

3. Override DidSetComputedStyle for the frame, so that we are aware of the CSS change.

Something on the SMIL part also needs change:

1. Correctly update animVal (Bug 1549164).

2. Update SMILCSSProperty::IsPropertyAnimatable for all geometry properties.

Comment 20

[violet.bugreport]

Attachment: Bug 1383650 - Map SVG geometry attributes to CSS

This patch maps SVG geometry attributes to CSS property, so that the
values set via SVG attribute will be known by CSS.

It doesn't deal with how the value is used.

Comment 21

[Robert Longson [:longsonr]]

Note that soft code freeze for Firefox 68 starts on May 6th (https://groups.google.com/forum/#!topic/mozilla.dev.platform/hjOlodOe5GQ). Might want to think about landing this after May 13th.

Comment 22

[violet.bugreport]

Attachment: Bug 1383650 - Resolve SVG geometry metrics from CSS

This patch makes SVG retrieve metrics from CSS style.

It doesn't handle <svg> element because geometry properties for
outer <svg> element has been partially implemented long ago, it
needs special change.

It doesn't deal with the impact on SMIL.

Comment 23

[violet.bugreport]

Now we should deal with SMIL and the animVal, baseVal stuff. An important quote from SVG2:

https://svgwg.org/svg2-draft/types.html#DOMInterfacesForReflectingSVGAttributes (emphasis mine)

In SVG 1.1, the animVal attribute of the SVG DOM interfaces represented the current animated value of the reflected attribute. In this version of SVG, animVal no longer representes the current animated value and is instead an alias of baseVal.

Comment 24

[violet.bugreport]

Attachment: Bug 1383650 - Notify style system when SMIL animation changes length

Since SVG geometry attributes become CSS property, we need to update SMIL
so that those attributes are animated as CSS property.

animVal becomes an alias of baseVal per SVG2 (see Comment 23 in Bug 1383650),
both of which relect the CSS style. Code and test need to be adjusted.

Comment 25

[violet.bugreport]

Attachment: Bug 1383650 - Invalidate path cache when geometry changed via CSS

We cached the path of an element. Previously we only need to invalidate
the cached path if an geometry attribute is changed. Now we also need
to invalidate if the corresponding CSS is changed.

Comment 26

[violet.bugreport]

After looking into the details in dom/smil as well as checking Chrome behavior, I'm wondering if the spec makes sense to alias animVal and baseVal.

I think a reasonable approach is to mostly keep the existing behavior: The animation of x is still based on SVGAnimatedLength::SMILLength, and both animVal and baseVal shouldn't change behavior. The only change on SMIL part is to notify the style system when SVGAnimatedLength::SMILLength::SetAnimValue() is called, so that the cascading will take place and the geometry element will get up-to-date style. And that's all.

Chrome seems to be using this behavior as well. One important thing I think is that existing html based on animVal won't break. And animVal will still contain some useful information about the SMIL animation which will be lost if only the computed style is exposed.

Brian, what do you think?

Comment 27

[violet.bugreport]

Attachment: WIP-smil-patch.patch

This is a completely broken WIP patch just to illustrate the idea. (the servo api function doesn't seem to support this property, but I think some supporting api exists :))

Comment 29

[Brian Birtles (:birtles)]

(In reply to violet.bugreport from comment #26)

After looking into the details in dom/smil as well as checking Chrome behavior, I'm wondering if the spec makes sense to alias animVal and baseVal.

I think a reasonable approach is to mostly keep the existing behavior: The animation of x is still based on SVGAnimatedLength::SMILLength, and both animVal and baseVal shouldn't change behavior. The only change on SMIL part is to notify the style system when SVGAnimatedLength::SMILLength::SetAnimValue() is called, so that the cascading will take place and the geometry element will get up-to-date style. And that's all.

Thanks for wading through all this. That sounds right to me, so I hope it proves to be that simple.

Chrome seems to be using this behavior as well. One important thing I think is that existing html based on animVal won't break. And animVal will still contain some useful information about the SMIL animation which will be lost if only the computed style is exposed.

Brian, what do you think?

If aliasing animVal to baseVal doesn't make implementing this any easier then it's better not to. We can do that in a separate bug later if needed.

Given that no other browser appears to have made the animVal change yet, there's a compatibility risk involved in being the first. If we don't need to take the risk yet, then it's best not to or at least to do it in a separate bug so that we don't have to back out the other changes in this bug if Web compat issues arise over the animVal changes.

Thanks again for all your work on this.

Comment 30

[violet.bugreport]

Attachment: fake-patch.patch

I'm implementing the SMIL part of this patch, when I found the Servo API Servo_DeclarationBlock_SetLengthValue() seems to be lacking support for geometry property.

Some context is in Comment 29. Basically when a length is animated via SMIL, I want to notify style system that the SMIL Override style has changed. I have the float value of the length and its unit, so I found Servo_DeclarationBlock_SetLengthValue().

I wrote a testing patch to see if it basically works, it turns out does. The animation on width works fine.

However, from https://searchfox.org/mozilla-central/source/servo/ports/geckolib/glue.rs#4540, it only supports width. And in my testing patch, if I change width to x, I got crash:

Hit MOZ_CRASH(stylo: Don't know how to handle presentation property) at servo/ports/geckolib/glue.rs

Emilio, is there any existing API better fitting this goal? If not, do I need to add geometry property support in Servo_DeclarationBlock_SetLengthValue()?

Thanks!

Comment 31

[violet.bugreport]

Forgot to needinfo... Please see the last comment (Comment 30)...

Comment 32

[violet.bugreport]

Attachment: patch-for-servo-glue.patch

I think this change on Servo glue is sufficient.

Comment 33

[Emilio Cobos Álvarez (:emilio)]

That and that patch seems reasonable... Though you may need percentages too?

Comment 34

[violet.bugreport]

Attachment: Bug 1383650 - Add geometry property support for Servo glue

These functions are useful to directly pass already parsed SVG
geometry property to CSS side.

Comment 35

[violet.bugreport]

Attachment: Bug 1383650 - Add conversion utilities to get CSS counterpart of SVG unit and AttrEnum

We need some utilities to convert SVG unit and attrenum to CSS unit and property id.
This is useful when we need to pass parsed geometry property directly to CSS.

Comment 36

[violet.bugreport]

Attachment: Bug 1383650 - Optimize attribute mapping by not parsing same thing twice

Geometry properties are the most used SVG attributes. When authors specify
them as attributes, we have to parse them in SVG side. So we don't want to
parse them in CSS side again, otherwise the introduced performance loss
is relatively high.

With this optimization, this feature implementation doesn't slow down
overall performace even if there are thousands of geometry elements.

Comment 37

[violet.bugreport]

Attachment: Bug 1383650 - Use CSS to layout foreignObject

Should also update layout code for foreignObject to use CSS geometry property.

Comment 38

[violet.bugreport]

Attachment: Bug 1383650 - Add reftest for rect, circle, ellipse and foreignObject

Comment 39

[violet.bugreport]

Attachment: Bug 1383650 - Run mach devtools-css-db to sync css db

This is the last part of this seris of patches to implement geometry property.
This particular patch just run ./mach devtools-css-db to update db per instruction
at the beginning of , and also a manual addition to the animation property db.

After this patch, the SVG geometry propery is implemented for <rect>, <circle>,
<ellipse> and <foreignObject>. We already implemented outer <svg>. Thus the
remainings are inner <svg> and <image>, which are kind of different to the
others, so they will be handled in some follow-ups. Note that these patches won't
impact old behavior of inner <svg> and <image>.

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55a283e793df
Support AllowQuirks::Always option in helpers.mako.rs r=emilio
https://hg.mozilla.org/integration/autoland/rev/70e74dd6b45d
Add SVG geometry property support in CSS r=emilio
https://hg.mozilla.org/integration/autoland/rev/18b030b31660
Map SVG geometry attributes to CSS r=longsonr
https://hg.mozilla.org/integration/autoland/rev/f1f7b4ad9547
Resolve SVG geometry metrics from CSS r=longsonr,emilio
https://hg.mozilla.org/integration/autoland/rev/7bc3bff991c4
Invalidate path cache when geometry changed via CSS r=longsonr
https://hg.mozilla.org/integration/autoland/rev/2b003d678c58
Add geometry property support for Servo glue r=emilio
https://hg.mozilla.org/integration/autoland/rev/e864696f6cf8
Add conversion utilities to get CSS counterpart of SVG unit and AttrEnum r=longsonr
https://hg.mozilla.org/integration/autoland/rev/a7b8e6460fb8
Notify style system when SMIL animation changes length r=birtles,longsonr
https://hg.mozilla.org/integration/autoland/rev/6730776560c0
Optimize attribute mapping by not parsing same thing twice r=longsonr
https://hg.mozilla.org/integration/autoland/rev/447c9248342b
Use CSS to layout foreignObject r=longsonr
https://hg.mozilla.org/integration/autoland/rev/0118148f1534
Add reftest for rect, circle, ellipse and foreignObject r=longsonr
https://hg.mozilla.org/integration/autoland/rev/4316d55f87be
Run mach devtools-css-db to sync css db r=emilio

Comment 41

[Emilio Cobos Álvarez (:emilio)]

I'd probably wouldn't have landed this just yet. This is a pretty big change and we're on the soft freeze period until the 20th...

Comment 42

[violet.bugreport]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #41)

I'd probably wouldn't have landed this just yet. This is a pretty big change and we're on the soft freeze period until the 20th...

Are you sure? Per Comment 21, the freeze period ended on May 13th. I've also followed the dev-platform link :longsonr has provided, it says the same:

we'd like to ask that any risky changes be avoided from May 6 until after
the version bump to 69 on May 13.

Comment 43

[Emilio Cobos Álvarez (:emilio)]

It was delayed one week due to the Add-on bustage. See https://wiki.mozilla.org/Release_Management/Calendar (Release Date: 2019-05-21).

Comment 44

[violet.bugreport]

Please backout the change (I don't know how to backout)... I wasn't aware that the soft freeze was delayed.

Comment 45

[Emilio Cobos Álvarez (:emilio)]

Sure! FWIW you can just ask the sheriffs on #developers on IRC, they're always helpful :)

<emilio> sheriffs: Could you back out bug 1383650? (reason: accidentally landing in the soft-freeze period)
<ccoroiu|sheriffduty> emilio: hi, sure
<emilio> ccoroiu|sheriffduty: thanks!
<ccoroiu|sheriffduty> emilio: yw :)

And no worries, it's understandable ^.^. Sorry for the hassle :(

Comment 46

[Cristina Coroiu [:ccoroiu]]

Backed out 12 changesets (bug 1383650) for landing in the soft-freeze period

Backout: https://hg.mozilla.org/integration/autoland/rev/1624c5a31917

Comment 47

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/55a283e793df
https://hg.mozilla.org/mozilla-central/rev/70e74dd6b45d
https://hg.mozilla.org/mozilla-central/rev/18b030b31660
https://hg.mozilla.org/mozilla-central/rev/f1f7b4ad9547
https://hg.mozilla.org/mozilla-central/rev/7bc3bff991c4
https://hg.mozilla.org/mozilla-central/rev/2b003d678c58
https://hg.mozilla.org/mozilla-central/rev/e864696f6cf8
https://hg.mozilla.org/mozilla-central/rev/a7b8e6460fb8
https://hg.mozilla.org/mozilla-central/rev/6730776560c0
https://hg.mozilla.org/mozilla-central/rev/447c9248342b
https://hg.mozilla.org/mozilla-central/rev/0118148f1534
https://hg.mozilla.org/mozilla-central/rev/4316d55f87be

Comment 48

[Emilio Cobos Álvarez (:emilio)]

Wait what? Were backed out.

Comment 49

[Emilio Cobos Álvarez (:emilio)]

Daniel, do you know what happened here? See comment 46 for the backout.

Comment 50

[Daniel Varga [:dvarga]]

Hi, my merge candidate was under the backout, but when the next merge was done it got backed out.

Comment 51

[Robert Longson [:longsonr]]

The freeze is now over: https://groups.google.com/forum/#!topic/mozilla.dev.platform/hjOlodOe5GQ

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2660d957f42
Support AllowQuirks::Always option in helpers.mako.rs r=emilio
https://hg.mozilla.org/integration/autoland/rev/ae39b084c0b1
Add SVG geometry property support in CSS r=emilio
https://hg.mozilla.org/integration/autoland/rev/b3152421e832
Map SVG geometry attributes to CSS r=longsonr
https://hg.mozilla.org/integration/autoland/rev/c6d19171ee12
Resolve SVG geometry metrics from CSS r=longsonr,emilio
https://hg.mozilla.org/integration/autoland/rev/bda0f207ac29
Invalidate path cache when geometry changed via CSS r=longsonr
https://hg.mozilla.org/integration/autoland/rev/db3f16ac6322
Add geometry property support for Servo glue r=emilio
https://hg.mozilla.org/integration/autoland/rev/af7326dc7145
Add conversion utilities to get CSS counterpart of SVG unit and AttrEnum r=longsonr
https://hg.mozilla.org/integration/autoland/rev/5514f80c4f48
Notify style system when SMIL animation changes length r=birtles,longsonr
https://hg.mozilla.org/integration/autoland/rev/94c0cc004981
Optimize attribute mapping by not parsing same thing twice r=longsonr
https://hg.mozilla.org/integration/autoland/rev/b05458d3fe67
Use CSS to layout foreignObject r=longsonr
https://hg.mozilla.org/integration/autoland/rev/f9ec440d652a
Add reftest for rect, circle, ellipse and foreignObject r=longsonr
https://hg.mozilla.org/integration/autoland/rev/21b3ac9c4917
Run mach devtools-css-db to sync css db r=emilio

Comment 53

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

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

Comment 54

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

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

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03d3b6d0eeca
followup: Regenerate property database again. r=bustage

Comment 56

[violet.bugreport]

Attachment: Bug 1383650 - Remove obsolete check that is wrong in SVG2

Comment 57

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d2660d957f42
https://hg.mozilla.org/mozilla-central/rev/ae39b084c0b1
https://hg.mozilla.org/mozilla-central/rev/b3152421e832
https://hg.mozilla.org/mozilla-central/rev/c6d19171ee12
https://hg.mozilla.org/mozilla-central/rev/bda0f207ac29
https://hg.mozilla.org/mozilla-central/rev/db3f16ac6322
https://hg.mozilla.org/mozilla-central/rev/af7326dc7145
https://hg.mozilla.org/mozilla-central/rev/5514f80c4f48
https://hg.mozilla.org/mozilla-central/rev/94c0cc004981
https://hg.mozilla.org/mozilla-central/rev/b05458d3fe67
https://hg.mozilla.org/mozilla-central/rev/f9ec440d652a
https://hg.mozilla.org/mozilla-central/rev/21b3ac9c4917
https://hg.mozilla.org/mozilla-central/rev/03d3b6d0eeca

Comment 58

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 59

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5242bd1605a
Remove obsolete check that is wrong in SVG2 r=longsonr

Comment 60

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/f5242bd1605a

Comment 61

[violet.bugreport]

Attachment: Bug 1383650 - Support geometry property for SVG image element

The only different part is to resolve intrinsic image size. This patch
implements explicit requirements of the spec, but an edge case is tricky.
It's not clear per spec what the intrinsic image size is for an SVG
without explicit width/height, something like:

<svg>
<image href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg'><rect width='40' height='90' fill='red' /></svg>"/>
</svg>

Chrome treats the intrinsic size of the href svg as the default size of
a replaced element (300x150), our image/VectorImage.cpp doesn't resolve
size in this case.

We can handle this particular case in some seperate bug if necessary, I think.

Comment 63

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0a0359fdadb
Support geometry property for SVG image element r=longsonr

Comment 64

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/b0a0359fdadb

Comment 65

[violet.bugreport]

Attachment: Bug 1383650 - Remove some redundant attribute observing logic

Since geometry property values are computed from CSS, which also
takes care of change hint via CalcDifference, we can just remove
some logic to observe attribute change for x, y, etc.

Comment 66

[violet.bugreport]

I'll close this bug after the above patch is landed. There is only one thing remaining: inner <svg>, but it has some problems:

1. It seems no other browser has implemented inner <svg> yet.

2. The spec has some problem. It says x:

Applies to: ‘svg’, ‘rect’, ‘image’, ‘foreignObject’ elements

It doesn't make sense to exclude <symbol> if <svg> is supported. At least our implementation shares a lot of code between <svg> and <symbol>, it'd be awkward to only support <svg>.
This is probably just a typographical problem in spec though.

3. There are quite some places we need to fallback to attribute because of the display:none problem. It looks ugly, especially if <symbol> (which is always display:none) is supported, DOM API getCTM(), etc. won't report correct result. A wrong result doesn't seem to be better than null. But if we just change to null, it may break old behavior even if no CSS is involved.

I'll file a new bug for inner <svg> stuff.

Comment 67

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0646b8b69e7
Remove some redundant attribute observing logic r=longsonr

Comment 68

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/d0646b8b69e7

Comment 72

[Sebastian Zartner [:sebo]]

I think this should at least be documented in the browser compatibility information of the different geometry properties.

Sebastian

Comment 73

[hrustlovro]

I would like to reopen this bug.

Just tested using css x and y properties on some SVG inner elements and they don't work in FF developer edition 105b9.

There is this codepen where you can see results, it works fine in chrome, but not in firefox.

Accordingto the spec in MDN https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/x#tspan:

Note: Starting with SVG2, x is a Geometry Property meaning this attribute can also be used as a CSS property for used elements.

Comment 74

[Emilio Cobos Álvarez (:emilio)]

That's a chrome bug. use pixels instead of numbers.

Comment 75

[hrustlovro]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #74)

That's a chrome bug. use pixels instead of numbers.

Thanks for clearing this up.

Comment 76

[totszwai]

Is there a regression going on?

The following rem value doesn't work.

<svg viewBox="0 0 24 24" fill="initial" xmlns="http://www.w3.org/2000/svg" font-size="1.3333333333333333rem" height="1rem" name="StatusStarFill" x="-8" y="21" class="gvt-icon StatusStarFill arc-1">
  <path fill-rule="evenodd" clip-rule="evenodd" d="M12 16.667L5 22l3-8-6-4.5h7.5L12 2l2.5 7.5H22L16 14l3 8-7-5.333z" fill="inherit"></path>
</svg>

Setting height to 1em work, but not with rem.

The same code is working fine in Chrome.

Comment 77

[Robert Longson [:longsonr]]

No, just bug 1287054