I've seen many confirm forms in my life.
I've yet to see a confirm form that does NOT implement this ugly workaround:
$output = confirm_form($form,
t('Are you sure you want to delete path alias %title?', array('%title' => $path['dst'])),
isset($_GET['destination']) ? $_GET['destination'] : 'admin/config/search/path'
);
(taken from path.admin.inc. Many more examples in core.)
Easy one:
1) Implement that logic directly into confirm_form().
2) Grep core for 'destination' (apparently not as many instances as you imagine) and replace those belonging to such ugly confirm_form() constructs.
Comment | File | Size | Author |
---|---|---|---|
#18 | drupal.confirm-form-destination.patch | 2.78 KB | sun |
#12 | drupal.confirm-form.patch | 2.4 KB | dawehner |
#9 | drupal.confirm-form-destination.patch | 2.43 KB | dawehner |
#7 | drupal.confirm-form-destination.patch | 1.33 KB | sun |
#3 | drupal_582456.patch | 2.67 KB | tobiasb |
Comments
Comment #1
mattyoung CreditAttribution: mattyoung commentedsub
Comment #2
dawehnerHere are the two instances, i filterd out by other results.
Comment #3
tobiasbmy version
Comment #4
dawehnerThis is definitive better then before.
tobiasb moved up the "?", because it saves going into is_array, if there is a destination. I don't think it makes sense to set query and argument, when having an destination.
Comment #5
wmostrey CreditAttribution: wmostrey commentedI double-checked if all instances in core were covered in Tobias' patch at #3 and they are. From what I can see it's good to go.
Comment #6
sunWe should still keep the parameters on separate lines. Ideally, also move the closing ); with proper indentation onto an own line.
- Please do not remove the blank line here.
- If we have a destination, then we probably need to parse $query and $fragment out of it, because url() only accepts those separately for internal URLs.
I'm on crack. Are you, too?
Comment #7
sunPlease merge this with your patch.
Comment #9
dawehnerThanks.
Fixed the style problems form #6 and merged the patch from #7.
Comment #11
sunNote that I'm writing a more generic helper function #578520: Make $query in url() only accept an array currently.
Comment #12
dawehnerThis version works again...
Comment #14
sunTagging absolutely critical clean-ups for D7. Do not touch this tag. Please either help with this or one of the other patches having this tag.
Comment #15
andypostSuppose #14 should check for page=X argument right not destination with path=X is broken
Comment #16
sunThis patch is actually trivial to do, now that we have drupal_parse_url().
But to pass tests, #600554: drupal_parse_url() doesn't work with clean URLs disabled is required. But anyhow, this patch could be prepared for that already.
Comment #17
sunAny takers?
Comment #18
sunRTBC if bot passes.
Comment #19
andypost+1 as commited #600554: drupal_parse_url() doesn't work with clean URLs disabled
Comment #20
sun@andypost: Did you review this patch? Is it RTBC?
Comment #21
andyposttested on a clean install - it works now with page=X argument too!
Comment #22
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!