1189024 - Set filterUnits and primitiveUnits to objectBoundingBox for Devtools "invert" filter

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Daniel Holbert [:dholbert]]

As noted in bug 1180048 comment 13, we're triggering some NS_WARNINGS when rendering filters.svg's "invert" filter for the first time (at least), due to it having percentages which apply to a frame which initially has 0 width and 0 height (until reflow has completed).

This is in part because it has the default "userSpaceOnUse" sizing behavior, which means x/y/width/height are in the coordinate space that the target-element is inside of:
 http://www.w3.org/TR/SVG11/filters.html#FilterElementPrimitiveUnitsAttribute

This filter *really* wants to have its units (x="0%" y="0%" width="100%" height="100%") apply to the target-element's bounding-box -- not to the coordinate space around the element.  So, it should request objectBoundingBox to opt-in to that sizing behavior.

(I'm not 100% sure, but I think objectBoundingBox and userSpaceOnUse effectively behave the same when SVG filters are applied to HTML content.  But we get to the eventual result in different ways, and in this case we trip over ourselves a bit [triggering this warning] when trying to evaluate the userSpaceOnUse behavior -- so directly asking for objectBoundingBox would be nicer there, in addition to being a bit more declarative about the intent.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8640656 [details] [diff] [review]
fix v1

Review of attachment 8640656 [details] [diff] [review]:
-----------------------------------------------------------------

I tested and all the icons are still being inverted properly in the light theme.

The weird thing is that I see in the docs on MDN:

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/primitiveUnits - "If the filterUnits attribute isn't specified, then the effect is as if a value of objectBoundingBox were specified."
https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/filterUnits - "If the primitiveUnits attribute isn't specified, then the effect is as if a value of objectBoundingBox were specified."

Based on Comment 0, maybe those docs need to be updated?

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> I tested and all the icons are still being inverted properly in the light
> theme.

Thanks!

> The weird thing is that I see in the docs on MDN

[Looks like you got the quotes/links swapped there]

Yeah, the MDN text on primitiveUnits there is incorrect. (It doesn't match the spec:
http://www.w3.org/TR/SVG11/filters.html#FilterElementPrimitiveUnitsAttribute
...or our implementation:
http://mxr.mozilla.org/mozilla-central/source/dom/svg/SVGFilterElement.cpp?rev=bd079aadd3fe&mark=40-40,42-42#40
...or the bolded default on the same page.

The text on filterUnits is correct, though (and matches our implmentation) -- so that means the |filterUnits="objectBoundingBox"| that I'm adding in the patch is unnecessary.  (I only put it in there for completeness, and because I initially got filterUnits and primitiveUnits mixed up.) I'll go ahead and drop that part before landing.

Thanks for the review!

Comment 4

[Daniel Holbert [:dholbert]]

(I'll fix the primitiveUnits MDN page, too.)

Comment 5

[Daniel Holbert [:dholbert]]

(Turns out the previous edit to the primitiveUnits MDN page was someone fixing up the default value elsewhere in the page -- bolding the correct default value -- but they just forgot to update the prose. :)  Fixed now anyway.)

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1859af04f202

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1859af04f202