I've noticed from print statement debugging that prepopulate is trying to alter all the forms on a page, e.g. ones such as search_theme_form, not just image_node_form, story_node_form etc. Although it's not been a problem for me so far, I imagine it could have unexpected consequences. In the attached patch, I'm therefore suggesting disallowing prepopulate by default.

CommentFileSizeAuthor
disallow-default.patch.txt1.28 KBegfrith

Comments

eafarris’s picture

eafarris commented

The problem I have with 'off by default' is that there's no UI for turning it on for forms that aren't node/add/$type, like admin/settings/site-information (for example). Not that this is necessarily a use case for this module, but things like that *are* possible and it's pretty cool.

I'm not going to 'wontfix' this just yet; let's discuss it. Can you give an example where this might be harmful, or have unexpected consequences?

egfrith’s picture

egfrith commented

I agree that changing defaults is undesirable, and I take the point about limitiing the possibilities of using prepopulate.

I can't think offhand of an example where there might be harmful or unexpected consequences -- to do that I'd need to study fields of different forms specified in drupal to see how many fields share names. I'll try to have a look at this later.

I suppose that if there are unexpected consequences, these could be reduced by specifying the form name in the url, e.g. something like:

http://www.example.com/node/add/image?edit_form[image_node_form][title]=...

I think it wouldn't be too hard to add code to handle this.

However, this doesn't deal with potentially harmful consequences. However I guess as long as prepopulate is only changing or adding #default_value elements, nothing too bad should happen. And whether harmful things can happen is probably independent of the type of form that is altered anyway.

eafarris’s picture

eafarris commented

It's been a few months... any more thoughts on this? This patch would be the complete opposite of the patch at http://drupal.org/node/101916.

egfrith’s picture

egfrith commented

I haven't had time seach exhaustively for possible name confilicts. Given the desire for prepopulate to be more general (see http://drupal.org/node/101916), and given that I haven't thought of any really bad things that changing the defaults would prevent, I'm happy for this bug to be closed.

add1sun’s picture

add1sun commented
Status: Needs review » Closed (won't fix)

marking won't fix since OP agrees.