thanks to http://drupal.org/node/100775, node_form_add_preview() is completely destroying any existing #prefix values on node forms when you preview. :(

attached patch is the smallest/cleanest solution i could come up with that:
a) works
b) should be safe for php5 folks
c) keeps all the node form stuff "close" to itself (i.e. the node preview comes first, then the form's existing #prefix (if any) then the node form itself...).

normally, i wouldn't call this critical, but chx made me do it. ;)

cheers,
-derek

p.s. i'm told #100775 was backported to 4.7.x, so this needs to be, too.

CommentFileSizeAuthor
#10 node_form_add_preview_save_prefix_4.7_patch.txt1.48 KBdww
#6 node_preview_reason.patch.txt1.47 KBheine
#4 node_form_add_preview_save_prefix.patch_1.txt1.29 KBdww
node_form_add_preview_save_prefix.patch.txt960 bytesdww

Comments

chx’s picture

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

Let me tell you what I feel in PHP.

function chx_s_bane($version) {
  switch ($version) {
    case '4.7':
       return 'checkboxes';
    case '5':
       return 'node_preview';
}
ChrisKennedy’s picture

ChrisKennedy commented

Looks good to me too. chx_s_bane(6) == 'menu'? 'menu' : 'previewing checkboxes';

dries’s picture

dries commented

Can we improve the code comments a bit? I.e. explain why we do it this way?

dww’s picture

dww
we/he/they
commented
StatusFileSize
new node_form_add_preview_save_prefix.patch_1.txt1.29 KB

how's this?

      // If the node form already has a #prefix, we must preserve it.
      // In this case, we put the preview before the #prefix so we keep
      // the #prefix as "close" to the rest of the form as possible,
      // for example, to keep a <div> only around the form, not the
      // preview. We pass the global $form_values here to preserve
      // changes made during form validation.
dries’s picture

dries commented

Why does the preview use prefix to begin with? I guess that is a bit of a hack?

The extra documentation is great, but it lacks a bit of a 'why' (background information). It would be great to explain in the code comments how the preview works, and why it piggybacks on the prefix stuff. Thanks dww.

I think the code is acceptable. Let's just use this opportunity to document it properly, and to make it accessible to non FAPI wizards. :)

heine’s picture

heine commented
StatusFileSize
new node_preview_reason.patch.txt1.47 KB

Not sure if this is sufficient, but its quite a novel already.

dww’s picture

dww
we/he/they
commented

Dries: for why, you must read http://drupal.org/node/100775 (or heine's latest version of the comment). i'm all for self-documenting code and better comments. ;) my natural tendency is to comment a lot, and i know it's a fine line w/ core to avoid "bloat" by putting in too much (e.g. "don't refer to issue nids, that's what cvs log is for..."). but, thanks for asking the right questions and being persistent...

Heine: thanks, that's lovely (IMHO).

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks for the great docs, folks.

dww’s picture

dww
we/he/they
commented

yay, thanks! updated the copy on s.d.o so i didn't have to have a locally patched version there... ;)

dww’s picture

dww
we/he/they
commented
Version: 5.x-dev » 4.7.x-dev
Status: Fixed » Patch (to be ported)
StatusFileSize
new node_form_add_preview_save_prefix_4.7_patch.txt1.48 KB

here's a patch for DRUPAL-4-7

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Patch (to be ported) » Fixed

applied

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)