825411 - clipPaths, masks, etc. with failing conditional processing attributes cause referencing elements not to be rendered

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: test

See the attachment.  According to the spec currently, the clipPath, mask and pattern should all be allowed to be referenced.  The spec doesn't say anything about filter and gradient elements, but I think they should be treated the same way.

I mailed www-svg about it: http://lists.w3.org/Archives/Public/www-svg/2012Dec/0102.html

Comment 1

[Robert Longson [:longsonr]]

Let's take clipPaths as an example. The specification is clear that ‘clipPath’ elements are never rendered directly; their only usage is as something that can be referenced using the ‘clip-path’ property.

So what does it mean when http://www.w3.org/TR/SVG/masking.html#EstablishingANewClippingPath says that clipPath supports conditional processing attributes and that http://www.w3.org/TR/SVG/masking.html#InterfaceSVGClipPathElement implements SVGTests?

Comment 2

[Cameron McCormack (:heycam)]

I think the spec is clearly contradictory, since http://www.w3.org/TR/SVG/struct.html#ConditionalProcessing explicitly says:

  ‘requiredFeatures’, ‘requiredExtensions’ and ‘systemLanguage’ attributes will have
  no effect on ‘mask’, ‘clipPath’, and ‘pattern’ elements.

Comment 3

[Robert Longson [:longsonr]]

Indeed.

So is the spec plan to remove SVGTests as an interface on clipPath, pattern etc. If we changed our implementation that way it would fix this testcase.

Comment 4

[Cameron McCormack (:heycam)]

Sounds good to me.  Once we discuss http://lists.w3.org/Archives/Public/www-svg/2012Dec/0102.html in a telcon and confirm that we're keeping the comment 2 interpretation, I'll update the IDL and blue element summary boxes in the spec.

Comment 5

[David Zbarsky (:dzbarsky)]

Now that Bug 825147 has landed, all the failing cases except gradient are rendered.

Comment 6

[Robert Longson [:longsonr]]

Attachment: patch

Comment 7

[Robert Longson [:longsonr]]

https://tbpl.mozilla.org/?tree=Try&rev=189f8403b7a8

Comment 8

[Robert Longson [:longsonr]]

Looks like the mochitest-other failures were some other issue that was reverted.

https://tbpl.mozilla.org/?tree=Try&rev=ffc2e09075c5

Comment 9

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/078434287ab8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/078434287ab8

Comment 11

[Cameron McCormack (:heycam)]

(In reply to Cameron McCormack (:heycam) from comment #4)
> Sounds good to me.  Once we discuss
> http://lists.w3.org/Archives/Public/www-svg/2012Dec/0102.html in a telcon
> and confirm that we're keeping the comment 2 interpretation, I'll update the
> IDL and blue element summary boxes in the spec.

We agreed in the telcon today that we should keep the current applicability of SVGTests and conditional processing attributes, and to make it explicit that they have no effect on <filter> and gradient elements.

Comment 12

[Robert Longson [:longsonr]]

That seems like the worst possible answer. It costs memory to implemeent SVGTests and yet they are useless on those elements. Perhaps we should just ignore the specification since SVGTests on those elements are not functional.

Comment 13

[Cameron McCormack (:heycam)]

Sorry, I think I wrote that wrong.  I meant to say we agreed to keep the current applicability of conditional processing attributes (i.e., they're not applicable to clipPath, etc.), and to have SVGTests implemented only on the elements that the attributes are applicable on.  Thus SVGTests is now no longer implemented on SVGClipPathElement, SVGPatternElement, and in fact all of the elements that were previously inheriting from SVGDefinitionElement.

I updated the spec earlier today to this effect:

https://svgwg.org/hg/svg2/rev/e5fb6a22e70b

Comment 14

[Robert Longson [:longsonr]]

Ahh, that's much better. Happy now :-)