Check font faces markup for objects within groups

[yassilah]

Currently, the "createSVGFontFacesMarkup" is only looking for fontFamilies among objects of the canvas. This PR allows it to check for fontFamilies within (nested) groups.

[asturur]

Suggested change

	fontPaths = fabric.fontPaths, objects = [];
	this._objects.forEach(function add(object) {
	objects.push(object);
	if (object._objects) {
	object._objects.forEach(add);
	}
	});
	fontPaths = fabric.fontPaths, objects = this.objects;
	this._objects.forEach(function add(object) {
	var subObjects = object._objects;
	if (subObjects) {
	objects = objects.concat(subObjects);
	subObjects.forEach(add);
	}
	});

i wonder if this version would be eventually more efficient, changing the array size once for all objects rather than each object. Just a curiosity.

[yassilah]

What do you mean changing the array size once and for all? Doesn't concatenating the objects array with all subObjects change the array size as well?

[asturur]

Hi @yassipad , we need to make a test for this fix/feature.
Do you know how to do?

[yassilah]

Hi @asturur, sure thing, i don't know how to do it though, do you have any guidelines?

[asturur]

there should already be some test around extracting font faces for svg.
modify the test in order to have a group with nested different fonts. please use 2 levels of nesting not just one.

[yassilah]

Hey @asturur, just pushed the new test, let me know if that's enough :) Also i noticed maybe another flaw in the process of checking for custom font-faces while running the initial test: if fontFamilies are only set in the "styles" attribute and not as a general property of the text object, it simply gets skipped because of this line:

if (obj.type.indexOf('text') === -1 || fontList[fontFamily] || !fontPaths[fontFamily]) {
    continue;
}

Shouldn't it rather be something like:

if (obj.type.indexOf('text') === -1) {
    continue;
}

if (fontFamily && fontPaths[fontFamily]) {
    fontList[fontFamily] = true;
}

There may be a reason for it not to be the case, just checking :)

[yassilah]

PS/ it could also be slightly faster by adding another condition to skip as in:

if (obj.type.indexOf('text') === -1 || (!fontFamily && !obj.styles)) {
    continue;
}

if (fontFamily && fontPaths[fontFamily]) {
    fontList[fontFamily] = true;
}

[asturur]

i think this parser switch is not more necessary since fabric 3, please if it works without, remove it and use just new DOMParsers()

[yassilah]

I'm moving the PR to another branch to fix the dist files issue on my other PR.
#6195

[asturur]

you could have checked from a commit.

git checkout ....goodcommit... dist

to recover the old dist folder.