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?

#9 book_export_html-return_404_on_non_book_nid-321063-9.patch1.07 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View
#6 book_export_html.patch1.36 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 31,510 pass(es). View
#3 book-module-fix-undefined-errors-321063-v02.patch1.92 KBKars-T
PASSED: [[SimpleTest]]: [MySQL] 24,767 pass(es). View
book-module-fix-undefined-errors.patch618 byteseMPee584
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch book-module-fix-undefined-errors.patch. View
Members fund testing for the Drupal project. Drupal Association Learn more


earnie’s picture

Status: Needs review » Needs work

Code looks correct. We need a test.

eMPee584’s picture

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

Status: Needs work » Needs review
1.92 KB
PASSED: [[SimpleTest]]: [MySQL] 24,767 pass(es). View

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

Status: Needs review » Needs work

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

Kars-T’s picture

Status: Needs work » Needs review



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

1.36 KB
PASSED: [[SimpleTest]]: [MySQL] 31,510 pass(es). View

Re-rolled against current D7 HEAD.

I can confirm the bug.

The patch fixes the bug.

pillarsdotnet’s picture

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

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

Status: Patch (to be ported) » Reviewed & tested by the community
1.07 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es). View

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

Gábor Hojtsy’s picture

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.