kogmbh/WebODF — pull request #562 by peitschie (2014-04-28 03:29:39+00:00)
Title: Ensure merged paragraph style names are well defined

Though getAttributeNS is now standardised as of HTML4 to always return null, according to MDN documentation browsers can return a variety of values for a non-present attribute (generally null or ""). Update the logic in TextController to always use "" for the merged style name as the inline comment suggests is the expected behaviour.

On an unrelated note, I'm a little unsure that making OpRemoveText depend on being able to distinguish an undefined paragraph style name vs. a "". Are we sure that most forms of serialization will meaningfully distinguish these two values? E.g., Would an XML serialization library potentially strip out empty attributes? Or is the deserialization likely to preserve this? Modern HTML5 has moved towards attributes having explicit true/false values (i.e., ```` rather than what used to be the norm of ````), which is significantly more robust against this type of error.

Two possibilities for improving this:

1. Create an explicit clearMergedParagraphStyleName: "true" or "false". In the op, clear if value === "true"
2. Move the paragraph style name setting to be a separate Op instead (e.g., OpSetParagraphStyle), which means that all confusion is avoided. If the paragraph style shouldn't be changed, no operation is created. If the paragraph style should be cleared, adapt OpSetParagraphStyle to handle this and create one of those.

In option 2, the "merge paragraphs & change style" scenario becomes a sequence of two operations: ``OpSetParagraphStyle(paragraph 1 bounds) => OpRemoveText(same bounds as in source)``