Rollback - PHP error because of typo in form.inc line 3025

made the change assuming it's correct

hmm corrected filename but nothing in it

The change outlined in the original issue looks good but fixes go into Drupal 8 first and are backported.

Yes, definitely. Thanks!

Adding backport tag.

First time ever committing a patch but this is tagged Novice so I figured I'd try!

This patch should be against origin/7.x and it should be almost the same as #3.

Here we go!

duplicate of #1399790: form_pre_render_conditional_form_element() uses $t which is never defined

Actually this issue has a patch which passed D7 tests, and the other issue has the incorrect fix. We don't need get_t() here.

Closed that one has duplicate and re-opening this one and restoring RTBC.

Patches need to be against d8 first and as per the other issue this needs test

Oh yes, sorry. Somehow because this got moved to D7 I had assumed this was already committed in D8...

Needs to be left as d8

Oh and needs work as it will need a reroll

why do we need a reroll? Patch in #3 is for D8, or am I missing something?

Sorry for that super-weird crosspost.
Re-uploading #3. If it does not apply, the testbot will set this to needs work, so back to RTBC.

hey

just a question, in what way is this RTBC?

I asked the same question to myself, and then reread the issue from start to finish, and the patch in #13 is indeed the patch from #3 re uploaded which was set to RBTC in #4. The confusion was created by the patch being unset from RBTC and moved to d7 in #5 when it should have been left as RTBC rather than being moved to d7 without the d8 version being committed

@marcingy yes I agree but i don't know why it was set to RTBC in #4?

As the newb who set this issue 7.x in #5, I would like to know what the correct procedure is for this sort of thing? I saw this issue tagged Novice and Needs backport to Drupal 7 so I figured this would be an easy first patch to try but apparently things didn't go so smoothly!

From reading this issue and the Backport policy it seems the current procedure for any issues found in 6.x or 7.x that may apply to 8.x is to change them to 8.x, patch them and set to needs review, after review set them to reviewed & tested by the community, and then ???

Obviously they need to be set to Ready to commit for 8.x at some point and then after that do they get changed to a 7.x issue? Maybe this is something that should be added to the policy. Speaking as a totally newb who would like to start helping with Drupal, I feel there needs to be some sort of clarification with this. Sorry if this is the wrong place to bring it up.

Hey, Kevin. yeah, it's kind of confusing. sorry about that.

Basically, issues need to get fixed in 8.x before they get fixed in 7.x. That way we don't introduce any regressions. So the normal workflow is to get the 8.x patch perfect, then get it committed, then backport whatever gets committed to 7.x either by direct backport or marking the issue "patch (to be ported)" if that doesn't work.

There's no harm in posting a 7.x patch before then, as long as you clearly mark it so and—this is the important part—name the patch with a "-do-not-test.patch" extension so that testbot doesn't blow up and mark the issue to "needs work". It's just not the general workflow.

However, changing the version selector before the patch gets committed to D8 creates a little bit of confusion because our workflow is that that normally only happens after the patch has been accepted upstream. It's good for issues to stay on 8.x-dev while they're still applicable for that branch because there are a lot more eyeballs on that stuff than 7.x stuff (D8 has 4 core committers, D7 has 1.25 :)).

So two things:

1) From now on, go ahead and leave the version selector at 8.x-dev when you upload D7 patches, but remember to name the D7 patches with a "-do-not-test.patch" extension if you do that
2) Or, consider only attaching a D7 backport to patches explicitly in a "patch (needs to be ported)" state. Those ones definitely need it; others might go through several more revisions until they're ready to go.

Hope that helps!

Thanks for the explanation!

Well actually, this is RTBC. :)

This still doesn't have tests, from what I can see. Though on IRC xjm confirmed that no other callbacks defined in hook_system_elements_info() have $t() around their descriptions, so the fix looks correct.

first it is and then it isn't and then it is and now it isn't? :)

Observe that it is #2and#3 that is correct with #3 as D8 patch.

Adding tests to push this simple fix forward.

Merging in #1279026: Bug due to missing $t declaration in form_pre_render_conditional_form_element.

Committed/pushed to 8.x, moving to 7.x for backport.

:)

git cherry-pick c804c2bc6737f406eafd3e <=> attached patches.

This looks like a straight forward re-roll.

Committed and pushed to 7.x! Thanks. :D

This fix is totally wrong!!! needs rollback
See details and fix at #1421410: $t not defined in form_pre_render_conditional_form_element()

Seriously? #7 says we don't need get_t(). What else could be wrong? Or at least please explain why.

and in #22.

Yeah, I don't get it either. A little help?

#7 and #22 says nothing... SO this should use get_t() because both theme('checkboxes') and theme('radios') could be used on install&update time when locale is not accessible. This been discussed in IRC and see patch #1421410-6: $t not defined in form_pre_render_conditional_form_element()

Also this functionality broken and not testable - title attribute is not used in render see #1421410-9: $t not defined in form_pre_render_conditional_form_element()

Commited patch should be rolled back and this isse should be marked as duplicate of #1421410: $t not defined in form_pre_render_conditional_form_element()

Ok, point seen. Rollling this back makes sense, then.

Commits to revert (or just use the patches):
D8: c804c2bc6737f406eaf
D7: ea8e74bbd494ce2f8d

Ok, thanks for the additional info, andypost, and for the nice Git commands, Niklas! :)

Reverted the D8 & D7 commits, marking as a dupe of #1421410: $t not defined in form_pre_render_conditional_form_element().