Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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?
Comment | File | Size | Author |
---|---|---|---|
#9 | book_export_html-return_404_on_non_book_nid-321063-9.patch | 1.07 KB | pillarsdotnet |
#6 | book_export_html.patch | 1.36 KB | pillarsdotnet |
#3 | book-module-fix-undefined-errors-321063-v02.patch | 1.92 KB | Kars-T |
book-module-fix-undefined-errors.patch | 618 bytes | eMPee584 | |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedCode looks correct. We need a test.
Comment #2
eMPee584 CreditAttribution: eMPee584 commentedwell i do not yet know anything about simpletest and doubt i will have the time to learn it within the next week.
Comment #3
Kars-T CreditAttribution: Kars-T commentedThis 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.
Comment #4
grendzy CreditAttribution: grendzy commentedhm, seems like the check should be in book_export() ?
Comment #5
Kars-T CreditAttribution: Kars-T commentedHi,
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().
Comment #6
pillarsdotnet CreditAttribution: pillarsdotnet commentedRe-rolled against current D7 HEAD.
I can confirm the bug.
The patch fixes the bug.
Comment #7
pillarsdotnet CreditAttribution: pillarsdotnet commentedtestbot is green
patch in #6 is (coding-wise) the same as #3
Testing on my site confirms both the bug and the fix.
Comment #8
webchickThanks for the fix, and for the test!
Committed to HEAD.
Marking down to 6.x.
Comment #9
pillarsdotnet CreditAttribution: pillarsdotnet commentedPatch for d6 is identical to patch for d7, except for the tests.
Comment #10
Gábor HojtsyLooks good, and patch seems to have a history of people testing it (with previous identical relative code changes), so committed. Thanks!