joint.util.breakText is broken in Firefox when an element is not visible (Fix suggested)

[KarolBuchta]

Version Rappid v1.7.0

The above mentioned function breaks when a textelement is not visible (display:none).

It's this line:

  // get line height as text height / 0.8 (as text height is approx. 0.8em
  // and line height is 1em. See vectorizer.text())
  var lh = lh || textElement.getBBox().height * 1.25;

You can reproduce this issue here: http://jsfiddle.net/sym3tri/kWWDK/

Resolutions and discussions can be found here: https://github.com/DmitryBaranovskiy/raphael/issues/21

// In Firefox, getBBox fails if the node is hidden var oldDisplay = node.style.display; node.style.display = "block"; var bb = el.getBBox(), dif = a.y - (bb.y + bb.height / 2); node.style.display = oldDisplay;

Best Karol

[kumilingus]

Hi Karol, this would still fails in FF if the parent element of the node is hidden (as per discussion). The only solution I can think of would be to clone the element and append it to document body and do the measuring again. I am not sure what about all the css rules that were applied to the original element though. Maybe you would have to apply all the computed css styles on that clone too.

var bbox;
try {
  bbox = textElement.getBBox();
} catch (e) {
 var clone = V(textElement.clone(true)); 
 var svg = V('svg');
// possibly we can position the svg offscreen to avoid flickering?
 svg.append(clone);
 document.body.appendChild(svg);
 bbox = clone.node.getBBox();
 svg.remove();
}

The other option would be to trigger a human friendly error to warn programmer to make sure the element he's measuring must be visible in order to compute the bounding box.

try {
  bbox = textElement.getBBox();
} catch(e) {
  throw new Error('breakText: method requires the measured element to be visible.');
}

Or did I miss anything?

[KarolBuchta]

Hi Roman,

i thought about it yesterday, and even implemented a different approach half way, until i recognized, that the first approach you mentioned is already used within this method bummer :-D. I just completely forgot that aspect. At the top of joint.util.breaktext there's following code:

  breakText: function(text, size, styles, opt) {

        opt = opt || {};

        var width = size.width;
        var height = size.height;
        var svgDocument = opt.svgDocument || V('svg').node;
        var textElement = V('<text><tspan></tspan></text>').attr(styles || {}).node;
        var textSpan = textElement.firstChild;
        var textNode = document.createTextNode('');

        textSpan.appendChild(textNode);

        svgDocument.appendChild(textElement);

        if (!opt.svgDocument) {

            document.body.appendChild(svgDocument);
        }

As one can see, the attributes from the textElement are applied to the clone. So in this case it's perfectly sufficient to simply make the element visible. We can use opacity:0 or visiblity:hidden to avoid flickering in any case (these two attributes keep the element invisible, but measurable).

var bBox;

try {
    bBox = textElement.getBBox();
} catch (error) {
    textElement.style.opacity = 0
    textElement.style.display = 'block';
    bBox = textElement.getBBox();
}

The inline style will override the svg attribute as well as css classes. We don't have to care about resetting to the previous state, as the element gets removed later anyway. We also don't have to take care about visibility of parents or computed styles, or the limiting size for the textElement. The later is the responsibility of the developer as per signature of the method.

I think this is the most applicable approach in this case.

A more generic approach, that i tried out, is to fetch the first parent of the element, that applies a property that makes the element having no width/height, then temporarily show the element as above, getBBox(), and reset to it's previous state, but stop the search at the root node of the element/shape itself. This approach could be done by using getBoundingClientRect in FF. This method will return zero width/height for an element that is hidden in the sense of no width/height. But it would only work in non webkit browsers. Also finding and showing the first parent would not guarantee, that there isn't another element "hidden" between node and first invisible parent, requiring to traverse down the dom hierarchy again. A limited size cache would be a good option in this case. Doing it like this would ommit the necessity to obtain computed styles of any kind and would work well on a flat dom hierarchy, as we can expect on elements/shapes in a diagramm. I just described this as it might be useful somewhere else.

Best Karol

[kumilingus]

Hi Karol,

how the textElement even becomes invisible? Is it only the case when we call the method with style argument { display: none }? If yes, can we do just the following? (I've also added the opacity: 0)

        breakText: function(text, size, styles, opt) {

            opt = opt || {};
            styles = _.omit(styles || {}, 'display');

            var width = size.width;
            var height = size.height;

            var svgDocument = opt.svgDocument || V('svg').attr('opacity', 0).node;
            var textElement = V('<text><tspan></tspan></text>').attr(styles).node;
            var textSpan = textElement.firstChild;
            var textNode = document.createTextNode('');

            textSpan.appendChild(textNode);

            svgDocument.appendChild(textElement);

            if (!opt.svgDocument) {

                document.body.appendChild(svgDocument);
            }

[KarolBuchta]

Hi Roman,

here's an example, how the element could be hidden (in the sense of not measurable, killing firefox), with three possibilities:

//Stylesheet
.element text{
    display:none;
} 

//Dynamic Markup
<g class="element">
    <text style="display:none" display="none"></text>
</g>

If the omit method omits also nested properties from the attrs object (the styles argument i our case), then this covers two cases: the display attribute itself and the inline style attribute as well, but not the stylesheet case. But anyway the omitting would fix the firefox behaviour for all use cases, where a developer works with dynamically displaying things within a shape.

But it would break the following use case: On the stencil we show shape miniatures, where the text is hidden by default by a stylesheet rule. If someone tries to use breakText for all shapes, this breaks the boot of the editor in Firefox. I did it, because i wanted to have multiline text, without foreignobject first, as our print system had problems with foreignObjects as well as other problems when parsing and manipulating in the backend. That's where i encountered the bug. For the stencil display:none is really necessary i think, as the miniature shapes would have a wrong size because text is not part of the scalable group and it would break the functionality of the "align on grid" method used for stencil shapes.

What i meant in the previous post is that when you set the style.display="block" this covers all three cases, because inline styles have higher precedence than stylesheet styles, as well as higher precedence than svg attributes itself. So with one line code three cases are covered.

Best Karol

[kumilingus]

The issue title says joint.util.breakText is broken in Firefox when an element is not visible. What element are we talking about here? joint.util.breakText() signature is (string, object, object, object). The only SVGTextElement the method is working with is textElement that is created and destroyed on the fly during the method execution.

As far as I can see the method itself can fail (in firefox) in the following cases: 1) CSS stylesheet rule that is way to general like text: { display: none }, svg text: { display: none } - as this would be applied to our textElement once appended to the DOM (edited: body). 2) provide display: none via textBreak style argument 3) provide your own svgDocument that is hidden (in the sense of not measurable) via textBreak opt argument 4) provide your own svgDocument that is measurable, but there is a CSS stylesheet rule that is being applied to our textElement once appended to provided svgDocument.

1) & 2) & 4) would be fixed by adding style.display = 'block' to the breakText method

e.g stylesheet rule .element text { display: none }` would not affect this method.

Are you using svgDocument option? How do you call textBreak method?

Best, Roman

[kumilingus]

Hi Karol, sorry I can already see the benefit of adding style.display = 'block' inside the textBreak method. But I am trying to understand what exactly happen in your case with stencil.

You either using svgDocument option and providing one of the stencil paper.svg (that is hidden because the entire stencil is hidden) or the error comes from a different place (e.g view rendering).

Best, Roman

[KarolBuchta]

Hi Roman,

sry if my description was not detailed enough, or the issue title was misleading. These are my first steps on github, although i develop for quite a time. It's also my first free time contribution.

Here is how i call the method, it's aequivalent to TextBlockView::updateContent() from your reference implementation.

text = joint.util.breakText(cell.get('label'), size, textAttrsLabel, {
  # measuring sandbox svg document
  svgDocument: this.paper.svg
});

Yes, i was using the svg option, and used the paper svg as an argument. But what i meant with stencils is this: bpmn.css around line 202

.stencil .elements text, .stencil-paper-drag .element text  {
    display: none;
}

This means if a developer uses breakText in his viewRendering implementation in a shape, in FF there will be an uncaught exception while the editor is bootstrapped (in this process the stencil is created), because the stencil uses the real programmed shapes.

Please note the following: Why you did not see this error in your implementation is, because you do a feature detection for foreignobject in the view rendering of your TextblockView, so the code path that leads to joint.util.breakText is never executed in Firefox in your implementation. Hence you could never see this issue.

Is the origin clear now?

I submitted this issue, because i thought this method should work as general purpose method, and the fix for the firefox bug was easy in my case. Just wanted to avoid that someone else runs into the same problem, and at the same time improve developer convenience of your library.

About your list point 3: yes, but i thought that the main paper should always be visible as a prerequisite, right?

Best Karol

[kumilingus]

Hi Karol,

sorry for the delay. Well, that explains why you were experiencing these troubles.

.stencil .elements text: { display: none; } gets applied to the textElement once appended to a stencil paper SVGElement.

This should be fixed in this PR.

the main paper should always be visible as a prerequisite, right

Correct, but only if you are using views (joint.dia.ElementView and joint.dia.LinkView) that calls el.getBBox() internally. So it's possible to write a custom view for Element and Link that does not measure elements in the DOM but computes all metrics within javascript (using model attributes like width, height, 'x' and 'y').

Best, Roman

[KarolBuchta]

Hi Roman, thank you a lot.

This was just merged, so the issue can be closed.

Best Karol