Widget & formatter methods typically receive an $entity, a $langode, and field values as $items:

<?php
FormatterInterface
::prepareView(array $entities, $langcode, array $items);
FormatterInterface::view(EntityInterface $entity, $langcode, FieldInterface $items);
WidgetInterface::extractFormValues(EntityInterface $entity, $langcode, FieldInterface $items, array $form, array &$form_state)
WidgetInterface::formElement(FieldInterface $items, $delta, array $element, $langcode, array &$form, array &$form_state);
?>

Those $entity & $langode params are unnecessary clutter:
- They can now be accessed through $items (already possible in HEAD, #2061331: Add getEntity() helper in FieldInterface / FieldItemInterface makes it a tad easier)
This is what all the rest of HEAD that works with Field / FieldItems does, having them handed out here as separate params (as if they were different from the ones that can be found in the $items) is a WTF.
- Very little widget / formatter code actually needs them (do stuff differently depending on the $entity or $langcode), and the code that does need them can get them.
- They obfuscate the simplicity of the methods tasks : "do your stuff on those $items"

The above could be simplified to just:

<?php
FormatterInterface
::prepareView(array $items);
FormatterInterface::view(FieldInterface $items);
WidgetInterface::extractFormValues(FieldInterface $items, array $form, array &$form_state)
WidgetInterface::formElement(FieldInterface $items, $delta, array $element, array &$form, array &$form_state);
?>
Files: 
CommentFileSizeAuthor
#45 formatter_widget_simplify-2075095-45.patch94.18 KByched
PASSED: [[SimpleTest]]: [MySQL] 59,220 pass(es).
[ View ]
#45 interdiff.txt17.57 KByched
#42 formatter_widget_simplify-2075095-42.patch87.38 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,742 pass(es).
[ View ]
#40 formatter_widget_simplify-2075095-40.patch87.38 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,911 pass(es), 6 fail(s), and 2 exception(s).
[ View ]
#40 interdiff.txt1.44 KByched
#38 formatter_widget_simplify-2075095-37.patch88.15 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#38 interdiff.txt9.43 KByched
#35 formatter_widget_simplify-2075095-35.patch86.91 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#31 formatter_widget_simplify-2075095-31.patch90.94 KByched
PASSED: [[SimpleTest]]: [MySQL] 59,163 pass(es).
[ View ]
#26 formatter_widget_simplify-2075095-26.patch91.94 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,680 pass(es).
[ View ]
#25 formatter_widget_simplify-2075095-25.patch95.1 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,944 pass(es).
[ View ]
#23 formatter_widget_simplify-2075095-23.patch98.48 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,540 pass(es), 5 fail(s), and 1 exception(s).
[ View ]
#20 formatter_widget_simplify-2075095-20.patch103.31 KByched
PASSED: [[SimpleTest]]: [MySQL] 58,869 pass(es).
[ View ]
#20 interdiff.txt1.6 KByched
#14 formatter_widget_simplify-2075095-14.patch98.19 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,911 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#14 interdiff.txt4.92 KByched
#12 formatter_widget_simplify-2075095-12.patch96.2 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,783 pass(es), 36 fail(s), and 34 exception(s).
[ View ]
#12 interdiff.txt1.28 KByched
#10 formatter_widget_simplify-2075095-10.patch96.91 KByched
FAILED: [[SimpleTest]]: [MySQL] 58,035 pass(es), 38 fail(s), and 29 exception(s).
[ View ]
#10 interdiff.txt4.32 KByched
#8 formatter_widget_simplify-2075095-8.patch95.47 KByched
FAILED: [[SimpleTest]]: [MySQL] 56,748 pass(es), 217 fail(s), and 52 exception(s).
[ View ]
#8 interesting_interdiff.txt2.29 KByched
#5 formatter_widget_simplify-2075095-5.patch85.27 KByched
FAILED: [[SimpleTest]]: [MySQL] 54,014 pass(es), 742 fail(s), and 98 exception(s).
[ View ]
#5 interdiff.txt1.93 KByched
#4 formatter_widget_simplify-2075095-4.patch83.34 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

Issue tags:+Approved API change

Agree with the issue summary - doing this is a win for common sense and consistency

Assigned:Unassigned» yched

I started this

Assigned:yched» Unassigned
Status:Active» Needs review
StatusFileSize
new83.34 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Patch, let's see what the bot says.

#2061331: Add getEntity() helper in FieldInterface / FieldItemInterface would really make things nicer btw :-)

StatusFileSize
new1.93 KB
new85.27 KB
FAILED: [[SimpleTest]]: [MySQL] 54,014 pass(es), 742 fail(s), and 98 exception(s).
[ View ]

Fix method calls in Field UI (default value widget)

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-5.patch, failed testing.

Hah - of course, this is why it hasn't been done as part of #2021817: Make widgets / formatters work on EntityNG Field value objects..

So the patch results in widgets being placed at $form['field_foo']['en'] instead of $form['field_foo']['und'] for non-translatable fields in node forms, which breaks a lot of drupalPost() calls in tests.

That's because field.multilingual considers the field language to be 'und' (field is not translatable), while the entity language (for a node created *in a form*) is 'en'. I opened #2076445: Make sure language codes for original field values always match entity language regardless of field translatability about this.

So the language currently passed by field_invoke_method() / field.multilingual to widget methods *is* actually different from the language that can be found in the FieldItems for node forms. By using the latter rather than the former, the patch changes the structure of node forms.

This makes me even more convinced that FieldItems should know their own language, rather than having a separate $langcode param passed along to code that work on them - and this is what the patch does.

But I'm very confused ATM as to how we go forward here. Multilingual entity logic is really in flux right now:
#1810370: Entity Translation API improvements is still fresh,
#2019055: Switch from field-level language fallback to entity-level language fallback is reshuffling things,
and field.multilingual.inc is considered mostly obsolete (#2067079: Remove the Field Language API) but still governs how configurable fields work right now.

We *could* manually change each test that posts to node forms to account for the new form structure ($form['field_foo'][$entity_language] rather than $form['field_foo']['und']), but I don't know if that would be the final intended behavior for field / entity translation...

Status:Needs work» Needs review
StatusFileSize
new2.29 KB
new95.47 KB
FAILED: [[SimpleTest]]: [MySQL] 56,748 pass(es), 217 fail(s), and 52 exception(s).
[ View ]

This
- injects the langcode in each Field value object,
- adds a getLangcode() method on Field & FieldItem
- uses this langcode in widgets & formatters instead of the langcode of the entity

The first two items deserve their own issue, but let's see how this goes.
No easy interdiff because of merges, but the interesting new code is attached.

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.32 KB
new96.91 KB
FAILED: [[SimpleTest]]: [MySQL] 58,035 pass(es), 38 fail(s), and 29 exception(s).
[ View ]

Posted a separate patch for Field/FieldItem::getLangcode() method at #2078625: Field/FieldItem value objects should carry their language
Meanwhile, this is also included in the patch here.

This should fix a bunch of fails - some places were still using the langcode of the entity.

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.28 KB
new96.2 KB
FAILED: [[SimpleTest]]: [MySQL] 58,783 pass(es), 36 fail(s), and 34 exception(s).
[ View ]

Status:Needs review» Needs work
Issue tags:-Field API, -Entity Field API, -Approved API change

The last submitted patch, formatter_widget_simplify-2075095-12.patch, failed testing.

StatusFileSize
new4.92 KB
new98.19 KB
FAILED: [[SimpleTest]]: [MySQL] 58,911 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

So, this is where things get funny.

The Entity API doesn't use the same language logic than field.multilingual.inc. So, by moving widgets to use the language determined by the Entity API for the $items objects, rather than the one determined by the old field.multilingual.inc, this patch changes the language keys found in the form structures.

- HEAD: widget for 'body' in a french node is at $form['body']['fr']
- patch: widget for 'body' in a french node is at $form['body']['und'],
since Entity API considers fields for the default language of the entity to be language neutral...

Patch thus needs to update the tests that post to the old form structures:
CommentLanguageTest, TranslationWebTest, NodeFieldMultilingualTestCase.

Note that this actually doesn't change anything, the langcode keys in the forms are basically useless now, and do not determine in which langode the submitted values are stored: WidgetBase::extractFormValues() assigns values into the $items object that gets passed to it, how the form is structured doesn't change a thing.

In fact, IMO:
- We should get rid of those langcode subkeys in the $form structure, we're not editing entities in multiple languages in the same form, core entity form controllers edit an entity in one single language. Within that language, some fields are translatable and some are not, that's it. It doesn't make a difference for the form structure.
- Even if some contrib module wanted to do "side editing in two languages in one form", it's then its responsibility to structure those forms accordingly.
At any rate, it's not the job of *widgets* to spit form $elements nested below a langcode subkey, as they do currently (see the end of WidgetBase::form()). Widgets generate "what it takes to edit this field", that's it. Multilingual structure, *if needed*, should be added on top of the result of $widget::form() by the calling code.

Removing those langocde subkeys would mean updating a lot more tests, and is outside the scope of this patch, though.

Status:Needs work» Needs review

Note: patch contains bits of #2061331: Add getEntity() helper in FieldInterface / FieldItemInterface, we probably need that other one to get in first.

Totally +1 on removing the langcode level in the form widget structures, it has always been a major source of troubles. I agree that for advanced use cases the implementing module should build its form(s) by assembling form widgets or possibily entire entity forms with their respective languages, as suggested in #1728816: Ensure multiple entity forms can be embedded into one form.

testbot robbed our tags in #13 - including "Approved API change" :-p

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.6 KB
new103.31 KB
PASSED: [[SimpleTest]]: [MySQL] 58,869 pass(es).
[ View ]

theme_field() fails to output anything if $items is empty, so TestFieldEmptyFormatter does need to add stuff in $items. That's sick, but not for this issue to solve. Reverted my refactoring of TestFieldEmptyFormatter, and opened #2083835: theme('field') displays no value on empty fields even if the formatter returned an output for this.

+++ b/core/includes/menu.inc
@@ -1275,31 +1274,13 @@ function menu_tree_page_data($menu_name, $max_depth = NULL, $only_active_trail =
+  // Load the menu item corresponding to the current page.
+  if ($item = menu_get_item($active_path)) {
     if (isset($max_depth)) {

Is this, and other changes in menu.inc, related to this patch ? I mean, sure we should remove the old router at some point, but that's not up to us ? :)

This feels like a missing rebase.

StatusFileSize
new98.48 KB
FAILED: [[SimpleTest]]: [MySQL] 58,540 pass(es), 5 fail(s), and 1 exception(s).
[ View ]

Yup, didn't merge between pulling 8.x and diffing with it I guess.
Reroll / fully merged.

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-23.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new95.1 KB
PASSED: [[SimpleTest]]: [MySQL] 58,944 pass(es).
[ View ]

Reroll after #2026339: Remove text_sanitize() since we have TextProcessed as an EntityNG computed property.
Cannot reproduce the fails in #23, assuming testbot fluke.

StatusFileSize
new91.94 KB
PASSED: [[SimpleTest]]: [MySQL] 58,680 pass(es).
[ View ]

FWIW, I think this should be ready now.

Status:Needs review» Needs work
Issue tags:-API change, -Field API, -Entity Field API, -Approved API change

The last submitted patch, formatter_widget_simplify-2075095-26.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+API change, +Field API, +Entity Field API, +Approved API change

StatusFileSize
new90.94 KB
PASSED: [[SimpleTest]]: [MySQL] 59,163 pass(es).
[ View ]

Indeed, both are modifying TestFieldEmptyFormatter.
Merged.

Status:Needs review» Reviewed & tested by the community

Went through this a couple of times, looks good.

We can update https://drupal.org/node/1805846 and https://drupal.org/node/1796000 after commit - and also the blog posts on my site :)

big +1 on it!

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Patch no longer applies.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new86.91 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Status:Needs review» Needs work
Issue tags:-API change, -Field API, -Entity Field API, -Approved API change

The last submitted patch, formatter_widget_simplify-2075095-35.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+API change, +Field API, +Entity Field API, +Approved API change

#35: formatter_widget_simplify-2075095-35.patch queued for re-testing.

I think the testbot is having a bad day.

StatusFileSize
new9.43 KB
new88.15 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Thanks @amateescu :-)

Updated patch to make use of the recently added getEntity() instead of getParent()

Could you leave the ER 'autocomplete_path' hunk to #2084081: ER autocomplete doesn't work which is already rtbc? :P

Edit: typo.

StatusFileSize
new1.44 KB
new87.38 KB
FAILED: [[SimpleTest]]: [MySQL] 58,911 pass(es), 6 fail(s), and 2 exception(s).
[ View ]

Oh, sure, wasn't aware of that other issue, sorry.
Left a comment over there, reverted my changes here (other than using getEntity() rather than $element['#entity']).

Status:Needs review» Needs work

The last submitted patch, formatter_widget_simplify-2075095-40.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new87.38 KB
PASSED: [[SimpleTest]]: [MySQL] 58,742 pass(es).
[ View ]

Reroll.

Can't seem to reproduce the fails locally.

Status:Needs review» Reviewed & tested by the community

Back to RTBC then

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Patch no longer applies.

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs reroll+Avoid commit conflicts
StatusFileSize
new17.57 KB
new94.18 KB
PASSED: [[SimpleTest]]: [MySQL] 59,220 pass(es).
[ View ]

Reroll
+ removes a bunch of now needless "use EntityInterface" statements in the formatter classes, the method signatures do not contain an entity anymore. Patch already cleaned up widgets similarly, but I forgot to do the same when updating formatters.

Title:Widget / Formatter methods signatures needlessly complexChange notice: Widget / Formatter methods signatures needlessly complex
Priority:Normal» Major
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

Committed and pushed to 8.x. Thanks!

This'll need a change notice.

Yay !

There are already change notices for the formatter / widget D8 APIs, so I think we want to update those instead of creating a new one ?
https://drupal.org/node/1805846
https://drupal.org/node/1796000

I'll do that later today.

+1 sounds good.

Title:Change notice: Widget / Formatter methods signatures needlessly complexWidget / Formatter methods signatures needlessly complex
Status:Active» Fixed
Issue tags:-Needs change record

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

rephrase