Not sure if this is a bug or should be a feature request.
The id specific hook_form_alter runs before non-specific hook_form_alter, this means you cannot target general changes to forms made by other modules.
eg
- menu.module places the menu fieldset on all the node form for all content types
- I don't want the menu fieldset on my custom node type so I use modulename_form_modulename_node_form_alter() to remove it
- modulename_form_modulename_node_form_alter() runs before modulename_form_alter() so the specific changes are lost
Surely the id specific form of hook_form_alter() should be run after the general one (specific changes being refinements so they need to happen later)?
This patch merely switches the order that the functions are run, it works for me.
Comments
Comment #1
t-dub commentedI was also confused by the execution order. It seems that the logical flow would be from general to specific. The menu elements aren't actually even be present in the $form object when modulename_form_modulename_node_form_alter() executes (which is relevant when you want to modify an element).
This same behavior is present in D6.
Comment #2
johnmph commentedSame problem here, you can use the general function with a test on the form_id to take only the good form but it will surely decrease performances.
Comment #3
stephenrobinson commentedYou need to install the util module, then you can set the weight of your module, usually to 10 for a custom module will get it to run last...
Comment #4
t-dub commentedSetting the weight of the module won't address this issue. We are talking about two different _form_alter() functions implemented by the same module.
Comment #5
dave reidWhy would your module use both hook_form_alter and hook_form_FORM_ID_alter to alter the same form??? This is easily solved by module weights.
Comment #6
dww@Dave Reid: re: "Why would your module use both hook_form_alter and hook_form_FORM_ID_alter to alter the same form???"
You wouldn't, but that's not the problem here... :(
"This is easily solved by module weights."
Sadly, no, it's not. No matter what the weights are, hook_form_alter() always runs after hook_form_FORM_ID_alter(), so changes in the more specific alter function are clobbered by those in the more generic. Therefore, any module that does its altering via hook_form_alter() effectively breaks the ability to use hook_form_FORM_ID_alter(), since the generic hook_form_alter() runs after the specific ones. I'm nearly positive taxonomy.module does this, so any module that wants to alter anything about the taxonomy form elements needs to use hook_form_alter(), too. There are many other examples. This has already nailed me so many times in practice, both for sites I'm building and contrib modules I maintain, that I've basically given up on hook_form_FORM_ID_alter() at this point, and just always do things via hook_form_alter(). :(
Comment #7
20th commentedI agree that the order should be switched. I experienced problems not only with Taxonomy, but also with CCK and some other modules.
I thinks the hook_form_FORM_ID_alter() was designed to remove code from hook_form_alter(), make it smaller and easier to understand. But the wrong order makes hook_form_FORM_ID_alter() almost completely useless, developers are forced to use hook_form_alter(), on top of that you also have to adjust the module weight.
I was very surprised to see the same order as in D6. Please fix this!
Comment #8
dwwBot is happy. Patch is small and obvious. I already explained why I'm strongly in favor of this change in #6... Please let's fix this before 7.0-alpha1. Thanks!
Comment #9
dwwAt tha_sun's request in IRC, I added an inline comment motivating this ordering. Here's the IRC log:
Here's the comment I added:
Comment #10
sunThank you!
Comment #11
damien tournoud commentedBig -1.
A module *can* implement a generic alter function, but it *cannot* implement *all* the specific alter functions. In that sense, that change makes it impossible to override specific alter functions.
Comment #12
dwwNot true. If modules are targeting specific ids, and you need to override those, you set your weight higher and override them. Just like now.
All I know is in practice, I've wasted many hours of my life trying to use the specific functions to override stuff that ended up needing to be done via the generic handler, even though it was for a single form id. I'm now completely in the habit to never use the form-id-specific version since it never actually does what you need...
Comment #13
dwwBased on further IRC debate with DamZ and tha_sun, we agreed that we ultimately need 3 tiers here:
1) hook_form_FORM_ID_alter()
2) hook_form_alter()
3) hook_form_FORM_ID_post_alter()
Attached patch invokes the hook and adds full API docs to system.api.php
Comment #14
sunVery nice compromise.
Comment #15
dwwBetter title.
Comment #16
chx commentedTo quote ex_article.module
so a big and resounding YES to this patch.
Comment #17
alb commentedsmall question regard drupal 6.x, think also is for D7;
when I use unset inner a function namemodule_form_alter(..){......}
php work more if I not specified on which $form_id to apply the unset?
this take more resource
1)
function namemodule_form_alter(&$form, $form_state, $form_id) {
if (isset($form['type']) && $form['type']['#value'] .'_node_form' == $form_id)
{ unset($form['comment_filter']['format']); }
}
of this?
2)
function namemodule_form_alter(&$form, $form_state, $form_id) {
unset($form['comment_filter']['format']);
}
----------
Comment #18
20th commentedSo, will this be committed to D7, or is this already too late, now that the alpha is released?
Comment #19
webchickCould someone explain why this is needed now, when we got through fine with Drupal 4.7, 5, and 6 without this, and especially without this new "post" alter hook, which smells like a serious hack?
I think #13 needs more details behind it for those who weren't privvy to the conversation. Right now, this seems more like a "Wouldn't that be nice?" than a "OMG critical" API change. If it's the latter, please explain why. If it's the former, let's move to D8.
Comment #20
chx commentedAngie, we got through 4.7 and 5 because the specific form_alter did not exist :) we introduced the specific hook in Drupal 6 and it's a big headache alone as dww said and I demonstrated but let me elaborate on that. If a module acts on node types and it's configurable which then it will use the generic form alter as it has no other chance. On the other hand, my site specific module wants to use for clarity the specific form alter only to find it can't act on those that use the generic form alter. At this point I need to implement the generic form alter and check around whether it's my form ... totally defeating the purpose of specific form alters to the point where as dww said they are pointless and simpler to not use them. So "ZOMG critical" indeed because without the post form alter hook the specific form id alter in D7 is just frustrating and useless.
Comment #21
dave reidIsn't more the problem here that we don't have a specific hook_form_FORM_ID_alter() than can be run on node forms? Like a hook_form_node_form_alter()? We *always* have to put those types of modifications in the generic alter.
Comment #22
dww@Dave Reid: That's also true, but that's not the only place where this nails us.
@webchick: This isn't a hack. We really need multiple tiers. If there are only 2 tiers, there's always going to be a case where the order of the two makes life more painful for you. The only way to solve this is 3 tiers: 1) specific, 2) general override, 3) specific override.
We need this now because we're porting stuff to D7 and finding (again) that the form-id-specific overrides are basically useless and confusing in 98% of cases, so we're trying to fix it. So, either we can fix it now with a small patch that has no chance of breaking any existing code, or we can postpone it for another 2 years. Your call. ;)
Comment #23
webchickSo what about removing hook_form_FORM_ID_alter, if it's so useless, and having just hook_form_alter() and weights instead, like other standard hooks?
We just don't use this _post_alter() thing anywhere else in core. It smells fishy to me.
Comment #24
chx commentedEvery heard the phrase "pour the baby out with the bathwater" (obligatory nephew photo)? Nowhere in core we have anything near the ubiquitousness of form_alter so obviously we need a more specific version of it which then drags in the post alter. Fishy or not, I think it's a good one.
Comment #25
catchhook_form_FORM_ID_alter() is more performant, because it doesn't run on every single page with a form. 100 hundred modules running hook_form_alter() isn't much fun.
I'm not sure why hook_form_FORM_ID_alter() couldn't be moved after the generic hook though.
Comment #26
sun@catch: That has been discussed to death - problem is that by simply switching both, you introduce the very same problem to the other hook.
We are now discussing this totally simple addition much longer than it took us to flesh out the most straightforward solution to solve the problem. I'm 100% sure that every Drupal developer and themer already had to find out about this awkward limitation the hard way.
This patch does not break anything and makes an API finally usable. This is becoming more important in D7, because themes are also able to alter forms now, and themes will likely want to solely use this post_alter hook, thus always acting after any other changes by modules.
Comment #27
catchWorks for me, also switching the order would be an API change for modules already relying on that ordering, adding the new hook is just an API addition.
Comment #28
webchickSorry, I still think this is D8 material.
Because:
- If we introduce this _post() pattern here, we have to also introduce it for our 80 gazillion other _alter() hooks, which means even more API changes in D7. Or we don't, and then we introduce a WTF here because people will expect these APIs to behave the same. Allowing pre/post hooks en masse definitely sounds like a D8 movement to me.
- The current situation is exactly the way it was in D6, so this does not represent a regression. It was annoying in D6, it'll stay annoying in D7.
- I see no evidence of a critical bug (as in, XXX completely fails if this patch doesn't get in) because obviously we are now 15 point releases into D6, and this hasn't been needed. vs., for example, the hook_entity_update/delete/etc. this was a clear string of things that would be nightmarish without those hooks.
Comment #29
catchOne thought, doesn't change 8.x status - the current hook could be renamed to hook_form_FORM_ID_build() (like hook_page_build()), then hook_form_alter() and hook_form_FORM_ID_alter() - that way it'd still be three hooks but save the 'post' thingy.
Comment #30
sunhook_form*_alter() is the most commonly alter hook throughout Drupal. And almost the only one with a specific hook_form_ID_alter().
AFAIK, the only other is hook_query_TAG_alter(), but for that, we don't know whether the same issue applies yet.
This trivial patch is an attempt to solve the problem through a straightforward, non-API-breaking hook addition.
If we want to find out whether this approach could work in a common way and could be applied to other alter hooks throughout Drupal in 8 (or not), we need to have at least one location in core to allow us to see, analyze, and understand the real world implications when this is going to be used by contrib modules and themes.
If it works in the wild, then we might simply extend drupal_alter() with another $id argument for D8 and make it always do specific->generic->post-specific.
If it won't, then we'll know that there's no way around a totally complex hook implementation weighing system.
I don't think anyone really expects APIs to stay the same. Contrary to that, this patch could even be backported to D6 -- if we would already be certain that it's the ultimate way forward.
I hope we can revisit the current decision.
Comment #31
webchickThe hook_X_build()/hook_X_alter() pattern is used for at least pages, if not other places. I think that's a suitable testbed for this pattern "in the wild" for the D7 cycle without making API changes to, as you say, the most frequently used hook in all of Drupal at this late stage.
Comment #32
dwwThis isn't an API change -- nothing about our existing API is changing at all. It's a trivial addition to our API to make life better for developers based on real world experience. Furthermore, if we decide it's not exactly the right model, we can always change it in D8. It's not like if we introduce this now it's set in stone forever -- the drop is always moving, right? ;) Anyway, I don't have the energy or desire to fight uphill battles like this. Knock yourselves out... Meanwhile, all of my modules will have giant switch statements in hook_form_alter() since hook_form_ID_alter() as it currently exists is almost never what you want or need. *shrug* It's not like people will be crushed to death at work by unsafe machinery due to this API WTF... We'll just have less performant code, and lots of developers will probably waste time trying (and failing) to get hook_form_ID_alter() to work as they hope.
Comment #33
Grayside commentedI just spent two hours trying to override a prepopulate_form_alter. This is why.
Comment #34
sunYeah, sometimes I wish we had a issue voting form that allows everyone to enter the amount of wasted hours.
Still RTBC, regardless of version.
Comment #35
chx commentedI move this back to D7 and will ask Dries to decide.
Comment #36
Frando commentedI also raise my support for this patch. I've been bitten by this quite a few times. While general _post alter hooks is something we can discuss in D8, hook_form_alter really is the only alter hook where I ever stumbled upon this, and there quite a few times already. Also, using hook_FORM_ID_post_alter() you can be rather sure on most forms to run last, which could likely solve quite a number of cases where modules currently had to set their weight to > 0. As this patch is so simple, doesn't involve any API breaks and makes life easier for developers, I think this could still easily be committed to D7. RTBC +1.
Comment #37
moshe weitzman commentedI support this for D7 as well.
Comment #38
effulgentsia commentedI support this too, though I haven't read the issue in detail yet to know what the objections are. I'll do so in the next day or so in order to add a more meaningful comment.
Combined with #692950: Use hook_module_implements_alter to allow modules to alter the weight of hooks in module_implements, that would be a lot of control.
Comment #39
effulgentsia commentedIt seems the objections to #13 are:
1) Is it needed in D7?
2) Does it create an unwanted inconsistency/WTF?
In relation to question 1, it was brought up that we've lived without this in D6 and so far in D7, so what's changed? Well, this issue was opened in August 2008, which is about when people started using D6 for production websites, and D6 was the first Drupal version to introduce a generic/specific alter hook option, so essentially immediately after people had the "do I implement the generic alter or the specific one" choice, the problem surfaced. As mentioned on this issue, people circumvented the problem by not using the specific one. But that was module developers, who are used to working around Drupal API deficiencies. In D7, we're expecting theme developers to implement form alter hooks (either the specific or generic ones), and chances are what they'll most often want is the specific one, and they'll expect it to run after modules are done messing with the form (they won't care whether the module implemented a generic one or specific one, they just want their theme to be last in making changes). How did we get this far into D7 without noticing this? Well, cause theme developers haven't been implementing specific form alter hooks yet, since that's what you do when working on a real site, not on Drupal core.
In relation to question 2, one issue is: does introducing a "_post_alter()" in one place mean we should introduce it everywhere alter hooks are used? Ideally, perhaps (just like one upgrade from D6 to D7 was creating 2 preprocess phases in the theme system: "preprocess" and "process"), but #13 only introduces "_post_alter()" for the specific alter hook, so we can leave the question of whether 2 alter phases are useful across the board to D8, and for D7, just stick to evaluating consistency among the places where we have the generic/specific pattern. In D6, the "form" alter hook was the only one that came with a specific option. But in D7, we now have 3:
hook_query_alter() followed by hook_query_TAG_alter()
hook_form_FORM_ID_alter() followed by hook_form_alter()
hook_block_view_MODULE_DELTA_alter() followed by hook_block_view_alter()
So, if we add hook_form_FORM_ID_post_alter(), seems to me that we should at the very least also add hook_block_view_MODULE_DELTA_post_alter(), but we'd still be left with an inconsistency relative to hook_query_alter(). But, HEAD already has that inconsistency in that for hook_query_alter(), the specific one comes after the generic one, so we wouldn't be adding a new inconsistency.
If we were willing to make a small API change, my proposal would be to make all three of these consistent with preprocess function order in the theme system:
In other words, first sort by module order, then for any given module, specific after generic. We could do this by passing $id (or an array of $ids, to handle the needs of hook_query_alter()) as the 5th param to drupal_alter(), and recode drupal_alter() to be able to handle that. This would remove the need for a "post_alter", and I'm willing to roll that patch if there's an agreement to do it for D7. If that's too big an API change to accept, then I remain in support of #13 for D7.
Comment #40
dwwRe: #39 @effulgentsia: Thanks for the thoughtful reply, much appreciated! However, I'm concerned about your final conclusion:
That's exactly what was originally proposed in this issue. See comments #9 through #13. In particular, #11 points out the problems of only doing generic then specific. Hence, the specific -- generic -- specific-post compromise in #13. In fairness, there was more IRC discussion about this than was summarized in this issue, but hopefully the above captures the essence of it. There *are* places were a module wants to try to alter all forms *after* most modules do specific altering. If we only have generic -- specific, then to override certain behavior, you must always know all the exact form IDs that are being altered by other modules. I'm not convinced this is that big of a problem, but DamZ was pretty adamant about it, so I won't fight it...
Cheers,
-Derek
Comment #41
effulgentsia commentedI think DamZ's concern in #11 was that even if a module was of higher weight than another one, it couldn't have a generic form alter override the lower weight's specific one. With my proposal, it could. Think of it this way: whether a module chooses to implement a specific function or a switch statement in a generic function should make no difference. The module with the higher weight should get to override decisions made at a lower weight.
Comment #42
sunAt this point, we cannot break APIs anymore. Adding a simple hook to fix this annoying issue and see how it goes is different though.
Comment #43
dwwRe #41: ahh, sorry, yes. I see what you're saying. That was also discussed, and deemed too large a change at this point. There's no way that's going to fly for D7. But, I hope that a simple addition for a "last chance to clobber a specific form" hook will still get in. Again, thanks for caring and participating!
So far, we've got 4 out of the 5 "Form system" maintainers currently proposed over at #621618: Revamp MAINTAINERS.txt supporting this patch: chx, effulgentsia, sun, and Frando. Plus, we've got a few Drupal greybeards like myself, catch, and Moshe. I'm not sure what else I can do to demonstrate the will of the core developer community that this is still a good idea for D7 (and could, indeed, even be backported to D6, although I doubt that's going to happen)...
I guess at this point, all that can be done is to await a decision from Dries. chx says he's pinged more than once. Let's hope for the best. ;) Given that I wrote the patch and have been soliciting reviews from prominent devs, I figure I should re-assign myself...
Cheers,
-Derek
Comment #44
effulgentsia commentedAs stated at the end of #39, I can definitely accept this. So in summary, yes, we're proposing a little inconsistency in order to avoid making a non-backwards-compatible API change that would create more consistency. The inconsistency isn't so bad. Basically, where we have the generic/specific pattern, D7 HEAD uses 3 different approaches to ordering, and we are proposing to maintain 3 different approaches, but to change one of those approaches to be different than it currently is:
1) For theme preprocess functions, there are 2 phases, each phase is ordered first by module weight, and within that, first generic then specific. No change to this proposed for D7.
2) For "form" and "block_view" alter hooks, HEAD has 1 phase, within that phase, we do first specific (ordered by module weight) then generic (ordered by module weight). We're proposing to change this to have 2 phases. The 1st phase same as HEAD, and the new 2nd phase to only be for specific (no "post" phase for generic).
3) For "query" alter hook, HEAD has 1 phase, within that phase, we do first generic (ordered by module weight), then each specific tag, first ordered by tag, then for each tag, ordered by module weight. No change to this proposed for D7.
Apologies if I'm being overly verbose above, but since one of webchick's main concerns in #28 has to do with introducing a "hacky" inconsistency, I think it's useful to provide more detail on exactly how this inconsistency relates to the inconsistency that already exists and that can't be fixed without breaking backwards compatibility.
Comment #45
effulgentsia commentedsorry. x-post, and i can't assign back to dww.
Comment #46
dwwno worries, re-assigning. ;) we should hopefully have #218066: Prevent cross posting from reverting metadata fields finished and deployed in the near future...
Comment #47
mikey_p commentedI have spent literally hours helping people in IRC trying to debug non-working form_alters only to have them explain that they are actually using form_FORM_ID_alter and not seeing their expected values. I strongly support this, or at the minimum a strong warning on the docs from hook_form_FORM_ID_alter. I don't think we really have alot of consistency with hooks in this regard to begin with and an ID specific hook is already unique, so adding another doesn't break any expected patterns. It seems to go quite nicely with the pattern of removing $op.
Comment #48
grendzy commented#13: 300481-13.form_id_post_alter.patch queued for re-testing.
Comment #50
dwwIt was just a hunk in system.api.php that no longer applied, presumably due to some other doc cleanups or something. Re-rolled.
Comment #51
effulgentsia commentedDiff'd with #13 to confirm that #50 is a re-roll only and has no substantive change. Back to RTBC.
Comment #52
mattyoung commented.
Comment #53
rfay+1 from me. This is a crazy, annoying WTF in D6. In D8, though, let's consolidate all of these so we don't have to have 3 hooks to do one thing. We can at least get it down to 2.
As I read it, this is also a no-risk patch.
We've all spent too much time chasing this one down in real projects. It needs to stop happening in D7.
Comment #54
chx commentedSo what's needed to get this? The people who support this includes dww, sun, chx, frando, moshe, effulgentsia. If that's not "reviewed and tested by the community" then I do not know what is.
Comment #55
damien tournoud commentedI case it wasn't clear since #13, I also support this.
Comment #56
merlinofchaos commentedWhile I am not completely comfortable with this solution, I do agree that a solution is needed, cannot be accomplished in contrib, and will work. I think it needs to be revisited from a big picture standpoint in D8, but the small picture is that this is a pain point in form manipulation that can be solved easily, albeit not elegantly, with this method.
Comment #57
drewish commentedI feel like effulgentsia's description of the problem and proposal in #39 was spot on. That said we've committed to freezing our APIs and I support sticking to that. We need to re-evaluate our mess of build/pre/post altering for D8 and come up with a consistent pattern. I share the concern that the naming is a bit "dirty" but I view this as a much needed band-aid that we can use to get through D7 since we'll likely be living with it for the next two years.
Comment #58
sunI've to admit that I previously didn't really parse eff's proposal in #39, sorry.
So... *gulp* ...in light of #757154: Base form_id via hook_forms() not taken into account for #validate, #submit, hook_form_FORMID_alter(), I guess we have two valid solution attempts for this issue:
$idargument todrupal_alter($type, $data, $context1, $context2, $id), whereas $id can be a single string or a list of strings. If that argument exists, invoke hook_TYPE_alter() and hook_ID_alter(), subsequently for each module, ordered by module weight.Effectively changing the order to: module1_TYPE_alter(), module1_ID_alter(), module2_TYPE_alter(), module2_ID_alter(), etc. -- i.e. at least a consistent and predictable order of hook invocations, driven by module weight, as always, and making themes come last.
Taking this and aforementioned issue as example,
which would ultimately lead to the following hook invocation stack:
whereas module hook invocations can be tweaked, as always, via module weights. Consistent and predictable.
Comment #59
chx commentedStop gap. Everyone says , let's do a quick band aid for D7. Dont rehaul drupal_alter this late.
Comment #60
catchYes, and we also have #692950: Use hook_module_implements_alter to allow modules to alter the weight of hooks in module_implements which is a much nicer solution than module weights to the hook weighting issue and still has a small chance of getting in, but which wouldn't work with the execution order outlined by sun.
Comment #61
quicksketchI personally don't see this as a critical deal-breaker for Drupal 7. Yes, it's annoying. That's about it. effulgentsia's proposed solution in #39 is the only one that makes a good amount of sense. Having 3 hooks simply further negates any performance benefit we got from creating the form-specific alter (a half-baked API mistake). And as others have noted, most modules are forced to use hook_form_alter() anyway because it's the only way to modify the node form, so we've already lost the performance benefit in all of those modules and actually made a performance penalty by looping through 2 sets of hooks instead of one.
This patch doesn't and can't fix the real problems we have with the order of our form_alter hooks. Even though effulgentsia supports this immediate change, I feel like we should wait and implement his #39 solution.
Comment #62
catchNot really, module_implements() is cached, so we only loop through modules on cache misses, and implemented hooks. The performance issue, if there is one, is on firing dozens of non-specific form alters on every form, there's no real performance penalty from the potential existence of hook implementations, although there was in Drupal 6. I see the specific form_alters() as a convenience thing, i.e. not having to do a big switch statement.
Comment #63
quicksketchRight, I'm not denying that the non-specific form alter is the problem. My point is that even after this patch, you're still likely going to be forced to use the generic hook_form_alter() anyway (to modify the node form or any other form derived from hook_forms()). Since you're going to be using it anyway, we haven't really solved anything and the generic hook_form_alter() still gets called for every form. If you've used hook_form_alter() even once, you've lost the benefit of having form-specific alters and there's no point in using them at all.
Though as I said, effulgentsia's #39 (and sun's follow up in #58) sound like excellent solutions which are both logical and actually have the performance benefit that the current API never really delivered. The hooks fire in a logical order and we don't end up making a "band-aid" fix that doesn't deliver any real improvement.
Comment #64
rfayIs it worth making "one last exception" to get a real fix into D7? I don't want to say this too loud, but there has been some outstanding API cleanup going on in other places that will probably make us very happy for a long development cycle. Hmm.
Comment #65
Frando commentedThis is not an API change, just an API addition. All Forms API maintainers agree that while this solution might not be perfect, it's the best we can get without breaking BC in D7 and that it is a huge improvement over D6. So let's just get this in.
Comment #66
rfay@frando, if you were responding to my question in #64, I was questioning whether we should go ahead and do #39 now.
Frankly, I would support that, if we wanted to bend the rules.
I still support this as is, and would like to get it in. It would just make things easier.
Comment #67
effulgentsia commentedIf anyone's interested, I posted a proof of concept patch for #39 in #765860: drupal_alter() fails to order modules correctly in some cases that addresses the concern in #60.
Comment #68
sunWe now have totally awesome looking patch over in #765860: drupal_alter() fails to order modules correctly in some cases
Comment #69
sunFriends, contrary to this one, the other patch is not a stop-gap solution, and applies a consistent hook invocation pattern to all drupal_alter() stacks using multiple alter hooks throughout core.
Much cleaner, much more consistent, absolutely predictable, and DX++.
I'm really inclined to mark this issue as duplicate.
Comment #70
dwwIf #765860: drupal_alter() fails to order modules correctly in some cases is committed, this should be "won't fix". If #765860 is postponed, this should be RTBC.
Comment #71
quicksketchI totally agree, maybe #765860: drupal_alter() fails to order modules correctly in some cases should be marked critical instead and we should close this one. The other issue is more sane and solves the same problem. I vote for duping this one.
Comment #72
dwwRight. But we're not closing this one until the other is committed. ;)
Comment #73
effulgentsia commentedI think we reached a point where Dries needs to decide between this one and #765860: drupal_alter() fails to order modules correctly in some cases. Getting either one in is critical. That one is better DX but potentially breaks BC (for any module currently relying on a higher weight hook_form_FORM_ID_alter() running before a lower weight hook_form_alter()). This one does not break BC but is acknowledged as an inelegant stopgap until we do something better for D8.
Therefore, marking both as RTBC and critical. If that one makes it, this one can be "won't fix". If this one makes it, that one can be "normal" and possibly bumped to D8.
Comment #74
catchI'm going to postpone this on #765860: drupal_alter() fails to order modules correctly in some cases - we know Dries has seen this one, so it's just a case of whether the other patch is preferable now. If that gets moved to D8 we can bump this back up.
Comment #75
sunThe other patch has been committed. Yay!
Comment #76
effulgentsia commentedHi all. Because of the upgrade to drupal_alter(), we were able to avoid a hook_form_FORM_ID_post_alter(): yay!.
But, we're still missing a very important hook: hook_form_view_alter(). Please review #798138: Add hook_form_view_alter(). Thanks!