Download & Extend

Book outline confirm form uses wrong arguments

Project:Drupal core
Version:6.x-dev
Component:book.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

The confirm form for removing a page from a book outline uses the wrong arguments.

Attached patch corrects the arguments.

AttachmentSizeStatusTest resultOperations
patch_159.txt811 bytesIgnored: Check issue status.NoneNone

Comments

#1

I see the bug also, and this patch fixes it. However, warning text like that used in function book_form_node_delete_confirm_alter() should be added as well if the page has children.

#2

Now notifies you that "Any child pages will be relocated automatically."

AttachmentSizeStatusTest resultOperations
patch_160.txt860 bytesIgnored: Check issue status.NoneNone

#3

I was thinking something more like this in terms of varying the description depending on whether the node has children.

AttachmentSizeStatusTest resultOperations
book_remove_confirm_3.patch1.18 KBIgnored: Check issue status.NoneNone

#4

Some additional fine tunning of the wording.

AttachmentSizeStatusTest resultOperations
patch_161.txt1.16 KBIgnored: Check issue status.NoneNone

#5

Status:needs review» reviewed & tested by the community

Looks fine, applies cleanly, and works. Much more informative than before.

#6

Status:reviewed & tested by the community» needs work

I would not say "This may make it difficult to re-create the existing hierarchy." is anywhere close to informative. This suggests an uncertain outcome of the action, which is not something one can decide to agree or not. Can we be more specific, ie. not uncertain?

#7

how about something like:

'%title has associated child pages, which will be relocated automatically to maintain their connection to the book.  In order to re-create the existing hierarchy, %title may be added again using the Outline tab, and each of its former child pages relocated manually.'

It's wordy, but perhaps more accurate.

#8

It is more accurate, but I needed to stop on the "in order to" part and reread multiple times to get it. I think you mean:

'%title has associated child pages, which will be relocated automatically to maintain their connection to the book. If you'd like to recreate the hierarchy used before, %title may be added again using the Outline tab, and each of its former child pages relocated manually.'

The "existing" word without (to me) proper time information mislead me actually. I am not a native English speaker, so the above might not be correct. Feel free to fix or come up with better ideas.

#9

'%title has associated child pages, which will be relocated automatically to maintain their connection to the book. To recreate the hierarchy (as it was before removing this page), %title may be added again using the Outline tab, and each of its former child pages relocated manually.'

that maybe?

#10

Yes, this sounds much better.

#11

Status:needs work» needs review

patch with that wording attached.

AttachmentSizeStatusTest resultOperations
book_remove_confirm_5.patch1.32 KBIgnored: Check issue status.NoneNone

#12

%title has associated child pages, which will be relocated automatically to maintain their connection to the book. To recreate the hierarchy (as it was before removing this page), %title may be added again using the Outline tab, and each of its former child pages will need to be relocated manually.

Slightly clearer in my opinion.

AttachmentSizeStatusTest resultOperations
patch_165.txt1.31 KBIgnored: Check issue status.NoneNone

#13

Status:needs review» reviewed & tested by the community

patch applies - looks fine and works.

#14

Status:reviewed & tested by the community» fixed

Thanks, committed!

#15

Status:fixed» closed (fixed)