1149516 - SVG shape stroke becomes invisible if stroke-dasharray property is 0

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Thomas Zimmermann]

Attachment: svgcircle.svg

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.104 Safari/537.36

Steps to reproduce:

Open the attached SVG file in Firefox 37 (beta channel).
It contains just two shapes, a circle and a path.


Actual results:

Firefox 37 doesn't display anything. 
The reason seems to be the "stroke-dasharray: 0" property. Removing it solves the issue and the shapes appear. 


Expected results:

Firefox 36, Chrome 41 show a path and a circle with a visible black 1 pixel-wide stroke.
This is also the correct behavior according to SVG spec:

'If the sum of the values is zero, then the stroke is rendered as if a value of none were specified'
http://www.w3.org/TR/SVG/painting.html#StrokeDasharrayProperty

Comment 1

[Robert Longson [:longsonr]]

Per the specification section you've quoted Firefox 37 is correct.

Comment 2

[Robert Longson [:longsonr]]

I.e the sum of the values is zero so you get no stroke.

Comment 3

[Thomas Zimmermann]

(In reply to Robert Longson from comment #2)
> I.e the sum of the values is zero so you get no stroke.

I don't think so. If you read on in the spec the definition of the value "None" for stroke-dashoffset:

none: Indicates that no dashing is used. If stroked, the line is drawn solid.

Comment 4

[Robert Longson [:longsonr]]

the **stroke** is rendered as if a value of none. and stroke="none" means don't draw a stroke.

Comment 5

[Robert Longson [:longsonr]]

I.e. it does not say the stroke-dasharray is rendered as if a value of none was specified which is what you seem to want it to say.

Comment 6

[Thomas Zimmermann]

(In reply to Robert Longson from comment #5)
> I.e. it does not say the stroke-dasharray is rendered as if a value of none
> was specified which is what you seem to want it to say.

Well, I assumed that in the section of the "stroke-dasharray" specification, the mentioning of the value "None" indeed refers to the property "stroke-dasharray". I have no reason to believe that the "stroke" property was meant.

I did some further research and found this comment of your's on a related issue, which seems to imply that stroke-dasharray="0" should indeed render a stroke:
https://bugzilla.mozilla.org/show_bug.cgi?id=885585#c9

Comment 7

[Jonathan Watt [:jwatt]]

(In reply to Robert Longson from bug 885585 comment #9)
> Per http://www.w3.org/TR/SVG/painting.html#StrokeDasharrayProperty
> 
> If the sum of the values is zero, then the stroke is rendered as if a value
> of none were specified
> 
> So that's why stroke-dasharray="0" works.

By "works", Robert means "works to reproduce the issue the bug report was about - i.e. not painting stroke".

Comment 8

[Jonathan Watt [:jwatt]]

Which makes sense in the context of the comment that came before that one, if not when the comment is taken out of context. ;)

Comment 9

[Thomas Zimmermann]

(In reply to Jonathan Watt [:jwatt] from comment #8)
> Which makes sense in the context of the comment that came before that one,
> if not when the comment is taken out of context. ;)

The comment before Robert's was (https://bugzilla.mozilla.org/show_bug.cgi?id=885585#c8):

> The path also draws correctly for me when stroke-dasharray="0"

I don't get what I'm understanding wrong, this clearly states that the path should be drawn in that case (and it did for FF < 37).

Furthermore, even if I'm wrong with interpreting the spec, could you explain me why FF37 suddenly behaves differently than previous FF versions & the other major browsers?

Comment 10

[Robert Longson [:longsonr]]

Raised a Chrome bug: https://code.google.com/p/chromium/issues/detail?id=472176

Comment 11

[Robert Longson [:longsonr]]

Firefox 37 changed because I fixed to obey the specification correctly in bug 1117514

It may well have also worked correctly i.e. not rendered anything when I made the comment in bug 885585 and then been broken again after that.

Comment 12

[Robert Longson [:longsonr]]

And a webkit bug https://bugs.webkit.org/show_bug.cgi?id=143269

Comment 13

[Thomas Zimmermann]

(In reply to Robert Longson from comment #11)
> Firefox 37 changed because I fixed to obey the specification correctly in
> bug 1117514

Thanks for the link, Robert. Still, I'm pretty confident that the spec, when saying 
"If the sum of the values is zero then the stroke is rendered as if a value of none were specified" 
is referring to the "stroke-dasharray" property, (thus meaning that the stroke should be rendered continuously). Why should it refer to the "stroke" property (which is defined several paragraphs above and not even linked to), when we are clearly located in the section explaining the "stroke-dasharray" property?

Comment 14

[Robert Longson [:longsonr]]

Take it up with w3c then: https://lists.w3.org/Archives/Public/www-svg/

Comment 15

[Jonathan Watt [:jwatt]]

The text:

  If the sum of the values is zero, then the stroke is rendered as
  if a value of none were specified.

could be read as:

  If the sum of the values is zero, then the stroke is rendered as
  if a value of none were specified [for the property under discusion
  right *here*, which is stroke-dasharray].

or alternatively as:

  If the sum of the values is zero, then the stroke is rendered as
  if a value of none were specified [specified for the stroke, i.e.
  for the 'stroke' property].

Seems like the spec definitely needs to be clarified. Like Robert, I had previously read this text as having the latter meaning. On reading the entire section through again, I now suspect it probably has the former meaning. It also probably makes more sense to have the latter meaning from the point of view of making properties/attributes in error behave as if they weren't specified at all.

Comment 16

[Thomas Zimmermann]

(In reply to Jonathan Watt [:jwatt] from comment #15)

> It also probably makes more sense to have the latter meaning from
> the point of view of making properties/attributes in error behave as if they
> weren't specified at all.

I assume you meant to say "It also probably makes more sense to have the *former* meaning", considering your message's context.

I've reached out to the www-svg mailing list for definitive clarification on this.

Comment 17

[Jonathan Watt [:jwatt]]

(In reply to Thomas Zimmermann from comment #16)
> I assume you meant to say "It also probably makes more sense to have the
> *former* meaning", considering your message's context.

Oops. Yes, indeed.

Comment 18

[Daniel Holbert [:dholbert]]

www-svg thread: https://lists.w3.org/Archives/Public/www-svg/2015Apr/0001.html

Robert / Jonathan may want to chime in; the one response so far matches Thomas's interpretation from comment 6 (which I can understand, given the spec language in question; really, this spec text should be clarified one way or the other, to remove ambiguity).

Comment 19

[Robert Longson [:longsonr]]

Attachment: patch

Looks like the specification is being clarified in such a way that we'll need to change.

Comment 20

[Robert Longson [:longsonr]]

Well done for not getting discouraged Thomas.

Comment 21

[Jonathan Watt [:jwatt]]

Comment on attachment 8587448 [details] [diff] [review]
patch

Thanks!

Comment 22

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4581040c895e

Comment 23

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=559f11400572

Comment 24

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2fe36ee71ed

Comment 25

[Phil Ringnalda (:philor)]

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

Comment 27

[Robert Longson [:longsonr]]

Comment on attachment 8587448 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1117514
[User impact if declined]: stroke-dasharray="0" renders as stroke="none" rather than a solid stroke. We've 3 bugs on this so far though so it does seem there is content that depends on this.
[Describe test coverage new/current, TreeHerder]: includes a reftest
[Risks and why]: low risk, just a couple of lines removed
[String/UUID change made/needed]: none

Comment 28

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8587448 [details] [diff] [review]
patch

Should be in 38 beta 3.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5662b0aa70fb

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/1dc6d70e9022

Comment 32

[Thomas Zimmermann]

Thanks a lot for resolving this so quickly, and for patching FF38 and 39, too!