bugfix - textcombine

[btsimonh]

fixes #232

[nigelmegitt]

This line will mask the impact of upstream code not setting _isd_element when we want that to happen, so possibly better not to add it?

[btsimonh]

Hmm, I'm not sure in what circumstances _isd_element would be unset, but almost certainly we'd not want to combine spans which had not come from the same source element. I think it adds robustness. If we just want to highlight errors as well as being robust, we could move the test inside the if, and console.error? no access to context.errorHandler there at the moment :(.

[nigelmegitt]

That's a good point @btsimonh , I hadn't thought of it like that.

[nigelmegitt]

Does the test at https://github.com/w3c/imsc-tests/blob/main/imsc1_1/ttml/textCombine/textCombine001.ttml expose this behaviour?

[btsimonh]

@nigelmegitt - gave a PR for imsc-tests to test this specific point.
Images need re-rendering....

[palemieux]

@btsimonh What was wrong with your original proposal #232 (comment) ?

[btsimonh]

What was wrong with your original proposal

I think @nigelmegitt wanted the code to highlight if _isd_element was not set, because it may clearly be an error?
Hence the move of the test - but no logging in that fn, so console.error is the best I could do without a lot more mods?

[palemieux]

I think @nigelmegitt wanted the code to highlight if _isd_element was not set, because it may clearly be an error?

AFAIK _isd_element is set only on elements that are available for merging. Wasn't that the intent @nigelmegitt ?

[nigelmegitt]

@palemieux Yes that was the intent, but I think we did not have test cases for textCombine and the code goes through a different path in that case.

It should not be the case that any elements that are candidates for merging do not have _isd_element so if that does happen we should have some mechanism for identifying it, like raising a console warning; at the same time we should fix those cases, and the elements that go through the textCombine path are one of the cases to be fixed, as I understand it.

[palemieux]

textCombine should not have _isd_element since it is not subject to span-per-character processing.

For the purpose of flagging things that should have _isd_element but do not have it, how would one differentiate an element that should have it but does not vs. an element that does not have because it should not have it.

[btsimonh]

can you highlight a case of 'should not have it' where the recombine function would get called?

[palemieux]

Replaced by #239