search

readium / readium-sdk

A C++ ePub renderer SDK
BSD 3-Clause "New" or "Revised" License
389 stars 164 forks source link

Error handling: should discriminate EPUB version 2 / 3 ? #133

Open danielweck opened 9 years ago

danielweck commented 9 years ago

The hard-coded error types do not seem bound to the version of the EPUB that is being parsed, which results in EPUB2 files being incorrect reported as invalid when missing some metadata that is mandatory in EPUB3: https://github.com/readium/readium-sdk/blob/develop/ePub3/utilities/error_lookup_table.cpp#L126 => to be verified, I am not sure!

danielweck commented 9 years ago

Related: https://github.com/readium/readium-sdk/issues/11 Jim said: "The ePub 3 spec says that property is required, as it's used in the generation of the publication's unique ID. It wasn't required for earlier versions of the spec though."

danielweck commented 9 years ago

Suggestion (1): let's review all ViolationSeverity::Major and ViolationSeverity::Critical error types, and let's decide whether some of them should be downgraded. This way, we do not have to refactor the error database to include an additional EPUB version field. Instead, we can just consider metadata errors as benign, especially if they do not affect the readability of a publication (unlike say, circular references of fallback references, which can definitely cause problems further down the line in the parsing / resource-fetching process). Caveat: that's okay for a reading system, but I suspect that an authoring tool based on the Readium SDK would benefit from "proper" (accurate) error reporting.

Suggestion (2): let's display the "warning" message (UI popup dialogs) only for ViolationSeverity::Critical (ignoring ViolationSeverity::Major), and let's downgrade errors which can be regarded as more "benign" to ViolationSeverity::Major (e.g. the EPUB2 missing metadata).