search

jwulf / press-star

A node-based implementation of the popular Death Star authoring environment for PressGang / Docbook
4 stars 1 forks source link

programlisting element without language attribute causes inability to validate/save #39

Closed xsgordon closed 11 years ago

xsgordon commented 11 years ago

When the editor encounters a programlisting element without a language attribute (which is valid and handled gracefully by Publican, csprocessor, and PressGang) clicking validate results in:

Communication error requesting validation: 0.

On the server side the output returned before app.js crashes is:

Checking 1 <programlisting> elements for language

/root/press-star/lib/topicdriver.js:279
                return cb(err, $.html());
                             ^
TypeError: Cannot call method 'html' of null
    at null.<anonymous> (/root/press-star/lib/topicdriver.js:279:34)
    at exports.each (/root/press-star/node_modules/cheerio/lib/api/traversing.js:64:24)
    at Object.adjustProgramlistingLanguages (/root/press-star/lib/topicdriver.js:253:25)
    at onXMLLintComplete (/root/press-star/lib/dtdvalidate.js:41:25)
    at ChildProcess.exithandler (child_process.js:538:7)
    at ChildProcess.EventEmitter.emit (events.js:91:17)
    at maybeClose (child_process.js:638:16)
    at Process._handle.onexit (child_process.js:680:5)
xsgordon commented 11 years ago

The offending block is:

    if (counter >= iterations && cb ) {
        if (_hasCDATA) { // don't rewrite it if it has CDATA, we'll lose it then
            $ = null;
            return cb(err, xml);
        } else { // rewrite if it has no CDATA
            $ = null;
            return cb(err, $.html());
        }
    }

To work around the issue for now I have made the second branch return XML as well rather than trying to access the html() function of a variable that at least in some cases is null. Appears to be working for now but probably has other impacts?

xsgordon commented 11 years ago

Looking closer I image the "other impacts" would be that my changes results in circumventing the point of that entire function, that being to rewrite incorrectly capitalized language values :D.

jwulf commented 11 years ago

Ah, i did that null assignment while tracking down memory leaks. Will look at it today.

On Thursday, May 30, 2013, Steve Gordon wrote:

Looking closer I image the "other impacts" would be that my changes results in circumventing the point of that entire function, that being to rewrite incorrectly capitalized language values :D.

— Reply to this email directly or view it on GitHubhttps://github.com/jwulf/press-star/issues/39#issuecomment-18644665 .

jwulf commented 11 years ago

OK, I'll fix this and refactor the validator into parts that can be meaningfully unit tested.

jwulf commented 11 years ago

Added an automated test: https://github.com/jwulf/press-star/commit/4f78e1446c54750fdad65d56ee6a91a1e3f2eb53