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?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’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
FileSize
1.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

Status: Needs review » Needs work

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

Kars-T’s picture

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

FileSize
1.36 KB

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
FileSize
1.07 KB

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.