Strict warning by field_ui_table_pre_render()

Trivial fix, removes about 10 PHP warnings on the field UI pages.

Is this covered by the existing Field UI tests when running them locally with E_STRICT, the test bots currently don't have that set so it wouldn't show up as a branch failure, but it'd be good to know whether we actually have the test coverage or not. See also #1211914: E_STRICT on test bots.

All this patch is doing, is creating a local variable that can be then be passed by reference into current(). Do we really want tests for this?

If you are referring to creating an independent issue that looks at the test coverage in this area, then yes these are needed. In D7, the form only gets touched by the menu and upgrade tests afaick and probably is not in a state that would trigger the warnings. I ran these tests locally without any notices.

So not to stall a trivial bug fix that is effecting live sites, moving back to RTBTC.

All this patch is doing, is creating a local variable that can be then be passed by reference into current(). Do we really want tests for this?

No, what we want, is to know whether there are tests that would trigger the bug that the patch fixes, that's not the same as writing tests specifically to check the E_STRICT issue. So, if those tests exist, then that's great, if not, I'd be fine with a follow-up issue rather than making this patch depend on adding them, but I'm not aware of one existing yet.

No, when running the Field UI tests locally with E_STRICT no errors show up. The problem isn't reproducible with just Drupal Core, because there is no way a field could end up indented. We'd have to create a test module to do that. I remember a similar problem in a different issue (that I couldn't find), so maybe it's indeed worth creating such a test module in a follow-up.

As of #7 I believe this is RTBC now, but correct me, if you like ;)

OK I opened #1593888: Tests for indentation in Field UI. Until #1211914: E_STRICT on test bots is done we can't add coverage that would actually catch this from the bot.

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

Here's the backported D7 patch.

Looks good :) Lets get this in.

Prior to the patch, I counted 42 individual warnings on one field UI screen!

There's an interesting discussion about this topic going on over at #1572394: Language detection by domain only works on port 80. Neither I nor other people could reproduce any E_STRICT problems with current(). (If you replace it with reset(), end(), prev(), next(), etc, then the E_STRICT warnings appear, which makes sense because those functions actually alter the passed-in array but current() doesn't.)

Curious if anyone knows why this is reproducible by some people rather than others - could it depend on PHP version?

In any case, the patch looks fine either way, so I committed it to 7.x. Thanks!

http://drupalcode.org/project/drupal.git/commit/e2bcdbc

Thanks David, good to see you using your newly acquired status :)

FYI:

Linux www.example.com 2.6.18-028stab085.3-ent #1 SMP Mon Mar 21 19:57:43 MSK 2011 i686
PHP Version 5.2.17
error_reporting 6143