What it does currently is put php errors in the log about $node->book being undefined and then dies horribly in the template. The attached patch is for D7 and cleanly applies to D6, does it still need a test to be written?

CommentFileSizeAuthor
#9 book_export_html-return_404_on_non_book_nid-321063-9.patch1.07 KBpillarsdotnet
#6 book_export_html.patch1.36 KBpillarsdotnet
#3 book-module-fix-undefined-errors-321063-v02.patch1.92 KBkars-t
book-module-fix-undefined-errors.patch618 byteseMPee584

Comments

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

Code looks correct. We need a test.

eMPee584’s picture

eMPee584 commented

well i do not yet know anything about simpletest and doubt i will have the time to learn it within the next week.

kars-t’s picture

kars-t commented
Status: Needs work » Needs review
StatusFileSize
new book-module-fix-undefined-errors-321063-v02.patch1.92 KB

This still applies.

To recreate:

Just install, activate the book module and load "/book/export/html/12345" and it will crash.

I remade the patch. With this we get a nice 404. And I added a test to the testBookExport() testcase.

grendzy’s picture

grendzy commented
Status: Needs review » Needs work

hm, seems like the check should be in book_export() ?

kars-t’s picture

kars-t commented
Status: Needs work » Needs review

Hi,

http://api.drupal.org/api/function/book_export/7

we would have to change this function a lot as the node is not loaded and there are no checks. book_export() is just a wrapper and loads us the real export function.

book_export_html() does all the stuff we need. I'd say not to blow up this issue and rewrite book_export().

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new book_export_html.patch1.36 KB

Re-rolled against current D7 HEAD.

I can confirm the bug.

The patch fixes the bug.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Needs review » Reviewed & tested by the community

testbot is green

patch in #6 is (coding-wise) the same as #3

Testing on my site confirms both the bug and the fix.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 7.x-dev » 6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks for the fix, and for the test!

Committed to HEAD.

Marking down to 6.x.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Patch (to be ported) » Reviewed & tested by the community
StatusFileSize
new book_export_html-return_404_on_non_book_nid-321063-9.patch1.07 KB

Patch for d6 is identical to patch for d7, except for the tests.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Looks good, and patch seems to have a history of people testing it (with previous identical relative code changes), so committed. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.