111317 - Need to implement SVG presentation attributes

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Håkan Waara]

We need to implement presentation attributes.  This will be done after sicking's
attribute rewrite.  A presentation attribute is, for example: 
|stroke-width=".02cm"|.

Right now to get this effect, you'd need to do:  |style="stroke-width:.02cm"|.

Comment 1

[Bradley Baetz (:bbaetz)]

-> new default owner.

This is one of the more important svg bugs we have.

Comment 2

[Bradley Baetz (:bbaetz)]

*** Bug 117914 has been marked as a duplicate of this bug. ***

Comment 3

[Mike Lee]

This bug could be fixed using xbl from my experiment. A proof of concept is
pasted below. It adds font-size and font-family attribute to <text> element.
This is one approach to it, I'm not sure if it's more efficent doing it this way
or have an xbl entry for each attribute. What do you think?

--cut--[svg.css]-- (from mozilla/res)
text {
  -moz-binding:url(svg.xbl#text);
}
--cut-------------

--cut--[svg.xbl]--
<?xml version="1.0"?>
<xbl:bindings xmlns:xbl="http://www.mozilla.org/xbl">
<xbl:binding id="text">
 <xbl:implementation>
  <xbl:constructor><![CDATA[
   var existingStyle = this.getAttribute('style');
   var fontSize = this.getAttribute('font-size');
   var fontFamily = this.getAttribute('font-family');
   if (!existingStyle)
     this.setAttribute("style", "");
   if (fontFamily)
     this.setAttribute("style", this.getAttribute('style') + ";font-family:" +
fontFamily);
   if (fontSize)
     this.setAttribute("style", this.getAttribute('style') + "; font-Size:" +
fontSize);
  ]]></xbl:constructor>
 </xbl:implementation>
</xbl:binding>
</xbl:bindings>
--cut--------

Comment 4

[Alex Fritze]

I think this is a good idea.
Some thoughts:

1.
We'd only need one binding for all the presentation attributes, which we can 
associate with all svg elements by using a universal selector:
* { -moz-binding:url(svgBindings.xml#presentationAttributes }

2.
If you set the presentation attribs in the constructor only, they will not be 
animatable. We need to set up mutation listeners that remap the attribs 
whenever they are changed. 

3.
Mapping into the style attribute will break serialization. E.g. if you have the 
element <circle fill="red"/>, it will appear as <circle fill="red" 
style="fill:red;"/> when serialized with the DOMSerializer.
We could map into a different stylesheet, but the interfaces are not in place 
yet.

4.
I wonder what the memory/speed overhead is.

Comment 5

[Bradley Baetz (:bbaetz)]

Do you get teh specicifty right doing it that way?

Who wins if you have a css rule for 'color: red', and a presentation atribute
for 'color=blue' ? What about if it were a local style rule instead?

Comment 6

[Mike Lee]

Its a proof of concept. Its my understanding presentation attribute got a higher
precedent of all other styling methods. I think using the element.style [1]
property is better approach than my string mangling.

[1] http://www.mozilla.org/docs/dom/domref/dom_el_ref27.html#1028210

Comment 7

[Mike Lee]

Attachment: code restructure

Added some optimisation as described in
news://news.mozilla.org:119/ajl5n7$[e-mail], however
this.attributes result in permission denied. Aint we suppose to have the enough
privilege?

Also did some rearranging, now all attribute inherit from one of the three
groups shape, global, and text (should I make it into one and off-load
additional property to child bindings. It should now be easier to maintain
(still experimenting with more than 1 level of inheritence, doesn't seem to
work).

I'll work on attribute listener, seems to me the only way is to implement a
get/set for each attribute. Any other suggestions?

Comment 8

[Mike Lee]

Hmm, maybe I should of applied back when they ask if I want additional bugzilla
privileges. I can't assign this to myself ;(

Comment 9

[Alex Fritze]

assigning to mike

Comment 10

[Alex Fritze]

The refactored code doesn't work, because you're messing up the prototype chain 
when attaching the same binding to different base tags. See my comment at the 
top of the file:
"We have to implement a separate binding for each svg element pending a fix to 
#41428"

Also could you please use tabs instead of spaces, because tabs are evil :-)

Comment 11

[Alex Fritze]

Marking this bug a dependence of #4128, because for an efficient implementation 
of presentation attributes in XBL we will need prototype-chain forking.

Comment 12

[Mike Lee]

Attachment: updated binding

change back the structure, because of bug 41428 :( added Interface SVGStylable
from alex and fix a small linking stuff. Also removed tab which I assume is
whaat alex is trying to say ;)

Comment 13

[chris]

Mike, presentation attributes do not have "a higher precedent of all other
styling methods" - ther reverse in fact. Presentation attributes have a
specificity of 000 in CSS2 (and thus, in SVG) very different from the 100
specificity of the style attribute.
http://www.w3.org/TR/REC-CSS2/cascade.html#q12
http://www.w3.org/TR/SVG/styling.html#UsingPresentationAttributes

And yes, its important to keep that specificity (though I note that all styling
seems to be broken, for example
http://www.w3.org/Graphics/SVG/Test/20011026/style-selector-BE-01-ps.html
and the following two tests).

Constructing a set of style rules and appending them to the UA.css might be one
implementation method.

Comment 14

[Mike Lee]

Thanks for informing me of this. It should be easy to fix the precedent order.
I'll fix it once I free myself from the current tasks on hand. 

I don't think current implementation support styling from external and style
within the style tags (only style attribute available). Alex can give a more
definate answer.

I'm not too familiar with the UA.css. Could someone enlighten me?

Comment 15

[Alex Fritze]

Ever since we introduced these xbl bindings I'm getting loads of subtle bugs. 
Most of them seem to occur only the first time an svg document is loaded; on 
all subsequent loads they disappear. This points towards problems associated 
with aynchronous loading of svgBinding.xml. 

Comment 16

[Alex Fritze]

bug#172574 is an example of one of those asynchronous-load xbl problems.
If it doesn't get fixed, we'll have to either remove the binding from the non-
root <svg> element or remove the 'display: block' declaration. In the latter 
case, percentage-based units on the width/height attribs of the <svg> element 
won't work anymore.

Comment 17

[Alex Fritze]

more xbl weirdness:

On first load, the tiger (http://www.croczilla.com/svg/tiger.xml) throws up 
a 'frame not found'-warning in nsCSSFrameConstructor when processing the 
constructor of the <g>-binding. Also the constructed frame tree contains two 
identical frames for each of the svg elements. Thus rendering takes twice as 
long, since each element is rendered twice. This is only if the tiger is the 
first svg file you're looking at after opening the browser. On a reload both 
the warning and the duplicate frames vanish.
It looks like xbl+svg needs some serious sorting out before we can switch on 
these bindings in builds... 
Maybe pre-loading svgBindings.xml would be a solution, but I'm not sure how 
that could be done.

Comment 18

[Mike Lee]

Currently all the constructors are at the top of the implementation. However
looking at all the other xbl usages, constructor seem to be placed at the bottom
of the implementation (after methods and property). If we move the constructor
to the bottom that might fix the aynchronous (if that is the problem) issue.
This is a probe in the dark, but it might be worth a try.

Comment 19

[Mike Lee]

Attachment: updated svg implementation to give style attribute priority

I tried moving the constructor to the bottom, seem to have no effect. The thing
is the style attribute is updated but the screen is not. Is there a way to
refresh the thing?

Comment 20

[Alex Fritze]

Dropping the dependencies on #41428 and #172574 since we now have a non-xbl
implementation of presentation attributes in the branch tracked by #182533.

Comment 21

[Alex Fritze]

Fixed by checkin for bug#182533.