Files: 
CommentFileSizeAuthor
#61 field-ui-1946404-61.patch105.09 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 55,408 pass(es).
[ View ]
#61 interdiff-from-53.txt2.77 KBtim.plunkett
#59 field-ui-1946404-59.patch105.73 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 55,497 pass(es), 17 fail(s), and 10 exception(s).
[ View ]
#59 interdiff.txt594 bytestim.plunkett
#56 interdiff.txt4.68 KBtim.plunkett
#56 field-ui-1946404-55.patch105.71 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 55,312 pass(es), 17 fail(s), and 122 exception(s).
[ View ]
#53 field-ui-1946404-53.patch105.18 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 55,800 pass(es).
[ View ]
#53 interdiff.txt1.4 KBamateescu
#52 field-ui-1946404-52.patch105.14 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#52 interdiff.txt1.78 KBtim.plunkett
#51 field-ui-1946404-51.patch105.13 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#51 interdiff.txt18.88 KBtim.plunkett
#46 field-ui-1946404-46.patch102.94 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 55,248 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#46 interdiff.txt862 bytesamateescu
#45 field-ui-1946404-45.patch102.94 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 55,397 pass(es).
[ View ]
#45 interdiff.txt898 bytestim.plunkett
#43 field-ui-1946404-43.patch102.06 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 55,302 pass(es), 1 fail(s), and 72 exception(s).
[ View ]
#43 interdiff.txt3.5 KBtim.plunkett
#40 field-ui-1946404-40.patch102.79 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 55,266 pass(es).
[ View ]
#39 field-ui-1946404-39.patch102.91 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 55,012 pass(es).
[ View ]
#39 interdiff.txt1.51 KBtim.plunkett
#37 interdiff.txt1.73 KBtim.plunkett
#37 field-ui-1946404-37.patch102.94 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 54,822 pass(es), 212 fail(s), and 257 exception(s).
[ View ]
#36 field-ui-1946404-36.patch101.91 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 54,741 pass(es).
[ View ]
#36 interdiff.txt2.17 KBswentel
#33 field-ui-1946404-33.patch101.67 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 54,712 pass(es).
[ View ]
#33 interdiff.txt1.41 KBtim.plunkett
#31 field-ui-1946404-31.patch101.36 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 54,732 pass(es), 5 fail(s), and 9 exception(s).
[ View ]
#31 interdiff.txt5.99 KBtim.plunkett
#24 field-ui-1946404-24.patch100.63 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 54,418 pass(es).
[ View ]
#24 interdiff.txt5.27 KBtim.plunkett
#21 field-ui-1946404-21.patch99.31 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 54,324 pass(es), 78 fail(s), and 108 exception(s).
[ View ]
#21 interdiff.txt9.19 KBtim.plunkett
#19 field-ui-1946404-19.patch91.53 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 54,313 pass(es), 127 fail(s), and 129 exception(s).
[ View ]
#17 field_ui-1946404-17.patch61.52 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#9 field_ui-1946404-9.patch284.16 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#7 field_ui-1946404-7.patch275.3 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#7 field_ui-1946404-7-do-not-test.patch61.4 KBtim.plunkett

Comments

Assigned:Unassigned» tim.plunkett

Working on this.

Assigned:tim.plunkett» Unassigned

Yeah, nevermind. The menu item is in a foreach, and I don't have the bandwidth to figure out EventSubscriberInterface to write a RouteSubscriber.

Would this be a route enhancer now? Any good examples of this already?

Assigned:Unassigned» tim.plunkett
Status:Active» Postponed

Okay, this actually wasn't hard because of the EventSubscriber, but because #1735118: Convert Field API to CMI isn't in yet. This will be much more straightforward with that in.

Title:Convert confirm_form() in field_ui.admin.inc to the new form interfaceConvert forms in field_ui.admin.inc to the new form interface

Working on this, it turns out that it will be very hard to do only the ONE confirm_form(), and leave the other forms, due to the foreach in hook_menu(). So this will now encompass the 6 form callbacks at once.

See the section "Dynamic Paths" in the change notice for how to port the foreach loop: http://drupal.org/node/1800686

(That is, unless it's possible to eliminate the foreach and make it a single, more flexible definition. That's always preferable.)

Status:Postponed» Needs review
StatusFileSize
new61.4 KB
new275.3 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Just to see what happens.
It's possible that breadcrumbs are completely broken for dynamic routes.

StatusFileSize
new284.16 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Hm.

From the testbot apache logs:

PHP Fatal error: Call to a member function id() on a non-object in /core/modules/menu/menu.module on line 209

Status:Needs review» Needs work
Issue tags:+Field API

tagging

Priority:Normal» Major

@jthorson I'm guessing that this is because of:

+  /**
+   * Overrides \Drupal\Core\Entity\Entity::id().
+   */
+  public function id() {
+    return $this->entity_type . '.' . $this->bundle . '.' . $this->field_name;
+  }

I had a problem apply the code to my git repo. I think a lot has changed recently.

@mgifford that code isn't anywhere near the menu module...

I'm working on rerolling this.

Thanks @tim.plunkett it's a big patch, thanks for doing the re-roll. But ya, you hadn't touched menu.module so was trying to look at what seemed like the next logical place.

Status:Needs work» Needs review
StatusFileSize
new61.52 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

So this is just a reroll because the last patch included the entire Field API as CMI conversion.

This needs a complete overhaul, because all of the routes are wrong and broken.

It will likely incorporate something like #1783964: Allow entity types to provide menu items or #1970360: Entities should define URI templates and standard links.

StatusFileSize
new91.53 KB
FAILED: [[SimpleTest]]: [MySQL] 54,313 pass(es), 127 fail(s), and 129 exception(s).
[ View ]

Ooookay. This was interesting.
It still needs a lot of cleanup, but I want to get it green first.

Issue tags:-FormInterface, -WSCCI-conversion
StatusFileSize
new9.19 KB
new99.31 KB
FAILED: [[SimpleTest]]: [MySQL] 54,324 pass(es), 78 fail(s), and 108 exception(s).
[ View ]

I think EnableDisableTest will still fail, but this is a start.

Um, bot, I wanted those tags

StatusFileSize
new5.27 KB
new100.63 KB
PASSED: [[SimpleTest]]: [MySQL] 54,418 pass(es).
[ View ]

Missed a spot, and figured out the EnableDisableTest

Yay & double yay !

Just began skimming through the patch, and I'm fairly new to the new routing system :
Can you expand a bit on the changes made on the menu paths building logic ?
Like :
- in hook_entity_bundle_info(), the 'path', 'bundle argument', 'access callback' entries are not needed anymore ?
- admin/structure/types/manage/article/fields/field_foo --> admin/structure/types/manage/article/fields/node.article.field_foo
Ouch :-/. Is there any way we could avoid putting config ids in URLs ?

As for the form callbacks themselves: is this just moved around, or did this require more significant changes ?

I didn't make any changes to the form callbacks, other than storing $instance as $this->instance, instead of putting it in $form_state['instance'], $form['#instance'], or just calling field_info_instance() a lot (all of which were done).

----

For Field UI paths, we currently have something like:

admin/structure/types/manage/%node_type/fields/%field_ui_instance/edit
It also has 'load arguments' => array($entity_type, $bundle_arg, $bundle_pos, '%map') in the hook_menu.

When visiting admin/structure/types/manage/article/field_body/edit
node_type_load() turns 'article' into an object, and field_ui_instance_load() takes the object and 'field_body' and the load arguments and loads a field instance.

In order to avoid adding more ParamConverters, I've switched the path to
admin/structure/types/manage/article/node.article.field_body/edit
since node.article.field_body is the ID of the FieldInstance entity

But then we have the bundle (article) repeated twice, and an uglier URL.

I'm not sure that routes can be bent to suit our original implementation, or if its worth it since ParamConverters are vaguely expensive

I'm A-OK with shifting the URI around to be easier. My main concern would be keeping breadcrumbs sane within the Field UI pages, which Tim informs me he was able to do. (I've not verified.) I didn't go over the patch in detail, but on a skim through it all looks fairly sane to me.

A few quick notes while I'm at it:

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php
@@ -0,0 +1,83 @@
+    foreach (entity_get_info() as $entity_type => $entity_info) {

entity_get_info() should be replaced with whatever its injectable modern version is.

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.php
@@ -0,0 +1,83 @@
+        $route = new Route("$path/display", array(
+          '_form' => '\Drupal\field_ui\DisplayOverview',
+          'view_mode' => 'default') + $defaults, array(
+          '_permission' => 'administer ' . $entity_type . ' display'));

The array merging syntax here took me a while to grok. Without PHP-aware syntax highlighting I can't really follow it. Changing the whitespace structure is probably sufficient to make it grokable.

I should go back through the classes looking for injectable versions of functions.

That Route syntax is CRAZY. I copy/pasted from somewhere else. Glad to know it's not important to someone if we fix it.

Not sure what to infer from the last exchanges with @Crell. Do we have a hope to get back to non-ugly URLs short-term ?
If not, I'm fine with getting this committed with its current URLs and see if this can be made better in a followup.

Thrilled at how this will make it easier to finally rename field_ui.module to entity_ui.module (since that's what it is, really).

(Still not a real review, though. Will try to do that asap)

core/modules/comment/lib/Drupal/comment/Plugin/Core/Entity/Comment.php:
+ *   entity_menu_base_path = "admin/structure/types/manage/{bundle}/comment",

That info is only correct if {bundle} is the node type, not the actual comment bundle name (comment bundles are 'comment_node_[node_type]'). I don't think that's the case ? So, not sure that works ?

+++ b/core/modules/field_ui/lib/Drupal/field_ui/OverviewBase.phpundefined
@@ -43,20 +43,17 @@
+    // Seriously, I hate comment module.
+    if ($entity_type == 'comment') {
+      $bundle = "comment_node_$bundle";
+    }

Yeah I added this to handle that, because WOW this is bizarre.

StatusFileSize
new5.99 KB
new101.36 KB
FAILED: [[SimpleTest]]: [MySQL] 54,732 pass(es), 5 fail(s), and 9 exception(s).
[ View ]

Reroll to address Crell's feedback. Still haven't fixed the comment hack.

StatusFileSize
new1.41 KB
new101.67 KB
PASSED: [[SimpleTest]]: [MySQL] 54,712 pass(es).
[ View ]

Fixed tests and clarified the workaround for comment module.

#33: field-ui-1946404-33.patch queued for re-testing.

StatusFileSize
new2.17 KB
new101.91 KB
PASSED: [[SimpleTest]]: [MySQL] 54,741 pass(es).
[ View ]

So, here's a solution for the comment hack, I'm not sure if this is the way to go or not, but it works at least.

StatusFileSize
new102.94 KB
FAILED: [[SimpleTest]]: [MySQL] 54,822 pass(es), 212 fail(s), and 257 exception(s).
[ View ]
new1.73 KB

Nice, I like that. Here it is with DI.

StatusFileSize
new1.51 KB
new102.91 KB
PASSED: [[SimpleTest]]: [MySQL] 55,012 pass(es).
[ View ]

StatusFileSize
new102.79 KB
PASSED: [[SimpleTest]]: [MySQL] 55,266 pass(es).
[ View ]

Reroll for the various entity/annotation commits.

Only got minor remarks :

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Routing/RouteSubscriber.phpundefined
@@ -0,0 +1,116 @@
+  /**
+   * @todo.
+   *
+   * @param \Drupal\Core\Entity\EntityManager $manager
+   *   The entity type manager.
+   */
+  public function __construct(EntityManager $manager) {

"Constructs a RouteSubscriber object." ?

Not for this patch either but: core seems to establish a kind of pattern that all subscriber classes to RoutingEvents::DYNAMIC are all called just RouteSubscriber ?

+++ b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.phpundefined
@@ -81,7 +81,9 @@ protected function attachLoad(&$menu_links, $load_revision = FALSE) {
-        $menu_links[$entity_id]->setRouteObject($route_objects[$route]);
+        if (isset($route_objects[$route])) {
+          $menu_links[$entity_id]->setRouteObject($route_objects[$route]);

Is that really needed ? Maybe would warrant a code comment ?

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Term.phpundefined
@@ -43,6 +43,7 @@
  *   menu_base_path = "taxonomy/term/%taxonomy_term",
+ *   entity_menu_base_path = "admin/structure/taxonomy/{bundle}",

Not for this issue, I guess, but those two entries using different placeholder syntaxes is a bit unfortunate.
Will that be tackled as a side effect of other, more general "mobe routes out of hook_menu()" tasks, or should we open a dedicated followup ?

Oh, sorry - while we're in there, if we could just reintroduce in class names and route names the sanity that was lost in the previous form_ids when we removed the "field settings" fieldset from the "edit instance" form:

Proposal:
class: FieldEditForm -> FieldInstanceEditForm
route: field_ui.edit.$entity_type -> field_ui.instance_edit.$entity_type

class: FieldSettingsForm -> FieldEditForm
route: field_ui.settings.$entity_type -> field_ui.field_edit.$entity_type

If not, no biggie, can be a followup.

StatusFileSize
new3.5 KB
new102.06 KB
FAILED: [[SimpleTest]]: [MySQL] 55,302 pass(es), 1 fail(s), and 72 exception(s).
[ View ]

I think the FieldInstanceEditForm name change makes things clearer, but as long as the URL is /field-settings, I think FieldSettingsForm is a clearer name. Follow-up to rename both the class and the URL seems fine to me.

Crell opened #1970360: Entities should define URI templates and standard links to address the various URI properties we have floating around.

I deleted the change to MenuLinkStorageController, since I think it may have just been an intermediate hack. If this fails, then we'll know what to make the comment :)

Yeah, I think everyone who needs a RouteSubscriber keeps copying the same code :)

Status:Needs review» Needs work

The last submitted patch, field-ui-1946404-43.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new898 bytes
new102.94 KB
PASSED: [[SimpleTest]]: [MySQL] 55,397 pass(es).
[ View ]

Welp, that would be why I had that check.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new862 bytes
new102.94 KB
FAILED: [[SimpleTest]]: [MySQL] 55,248 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Seems that @yched's concerns were adressed, so I just fixed the comment from the last interdiff and call this done :)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, field-ui-1946404-46.patch, failed testing.

Status:Needs work» Reviewed & tested by the community

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -194,6 +194,27 @@ class EntityType extends Plugin {
+  public $entity_menu_base_path;
...
+  public $entity_bundle_prefix;

the entity_ is redundant...

+++ b/core/modules/field_ui/lib/Drupal/field_ui/FieldOverview.phpundefined
@@ -571,7 +564,7 @@ public function submitForm(array &$form, array &$form_state) {
         field_create_field($field);
-        field_create_instance($instance);
+        $new_instance = field_create_instance($instance);
         // Make sure the field is displayed in the 'default' view mode (using
         // default formatter and settings). It stays hidden for other view
@@ -613,7 +606,7 @@ public function submitForm(array &$form, array &$form_state) {
         try {
-          field_create_instance($instance);
+          $new_instance = field_create_instance($instance);
           // Make sure the field is displayed in the 'default' view mode (using

Shouldn't this be using the injected plugin.manager.entity

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldDeleteForm.phpundefined
@@ -0,0 +1,92 @@
+    form_load_include($form_state, 'inc', 'field_ui', 'field_ui.admin');
+
+    $field = $this->instance->getField();
+    $bundles = entity_get_bundles();

It's a shame that we're not using an injected services from the container here... but AFAIK at the moment we can't inject things in ConfirmForms

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -0,0 +1,280 @@
+ */
+class FieldInstanceEditForm implements FormInterface {
+
...
+    $bundles = entity_get_bundles();
...
+    $additions = module_invoke($field['module'], 'field_instance_settings_form', $field, $this->instance, $form_state);

Shouldn't this implement the ContainerInterface and have a create method so that the plugin.manager.entity and the module handler can be injected

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldSettingsForm.phpundefined
@@ -0,0 +1,161 @@
+/**
+ * Provides a form for the field settings edit page.
+ */
+class FieldSettingsForm implements FormInterface {
...
+    $additions = module_invoke($field['module'], 'field_settings_form', $field, $this->instance, $has_data);
...
+    $field = field_info_field($field_values['field_name']);

and ContainerInterface here too... to inject modulehandler, field.info

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldWidgetTypeForm.phpundefined
@@ -0,0 +1,109 @@
+ * Provides a form for the widget selection form.
+ */
+class FieldWidgetTypeForm implements FormInterface {
...
+    $field = field_info_field($field_name);

and ContainerInterface here too... to inject field.info

Also chatting to @fubhy in IRC we should have followups in place once #1906810: Add a ParamConverter for upcasting TypedData objects and #1837388: Add support for upcasting parameters that depend on other parameters in a route so that we can reduce the redundancy of urls like admin/structure/types/manage/article/fields/node.article.body (which used to be admin/structure/types/manage/article/fields/body )

Status:Reviewed & tested by the community» Needs work

Crosspost with testbot :)

Thanks @alexpott, working on injecting stuff now.

Status:Needs work» Needs review
StatusFileSize
new18.88 KB
new105.13 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Okay, injected as much as I could, there is an issue with module_handler and $form_state, so just using \Drupal::moduleHandler for now.

StatusFileSize
new1.78 KB
new105.14 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

@amateescu pointed out a typo, thanks

StatusFileSize
new1.4 KB
new105.18 KB
PASSED: [[SimpleTest]]: [MySQL] 55,800 pass(es).
[ View ]

I just found another problem..

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldDeleteForm.phpundefined
@@ -70,7 +70,7 @@ public function submitForm(array &$form, array &$form_state) {
-      field_delete_instance($this->instance);
+      $this->instance->delete(TRUE);

Then, just delete() please (param is optional and defaults to TRUE).

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -37,7 +67,7 @@ public function buildForm(array $form, array &$form_state, FieldInstance $field_
-    $field = field_info_field($this->instance['field_name']);
+    $field = Field::fieldInfo()->getField($this->instance['field_name']);

Then should be injected as well ? ($container->get('field.info'))

See next comment.

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldSettingsForm.phpundefined
@@ -142,14 +143,14 @@ public function submitForm(array &$form, array &$form_state) {
-    $field = field_info_field($field_values['field_name']);
+    $field = Field::fieldInfo()->getField($field_values['field_name']);

Then should be injected as well ? ($container->get('field.info'))

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldWidgetTypeForm.phpundefined
@@ -41,7 +71,7 @@ public function buildForm(array $form, array &$form_state, FieldInstance $field_
-    $field = field_info_field($field_name);
+    $field = Field::fieldInfo()->getField($field_name);

Same
See next comment.

Actually, sorry:

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.phpundefined
@@ -37,7 +67,7 @@ public function buildForm(array $form, array &$form_state, FieldInstance $field_
-    $field = field_info_field($this->instance['field_name']);
+    $field = Field::fieldInfo()->getField($this->instance['field_name']);

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldWidgetTypeForm.phpundefined
@@ -41,7 +71,7 @@ public function buildForm(array $form, array &$form_state, FieldInstance $field_
-    $field = field_info_field($field_name);
+    $field = Field::fieldInfo()->getField($field_name);

Those two could now more simply be : $field = $field_instance->getField();

StatusFileSize
new105.71 KB
FAILED: [[SimpleTest]]: [MySQL] 55,312 pass(es), 17 fail(s), and 122 exception(s).
[ View ]
new4.68 KB

Oh duh, forgot that field.info was a service.

Also, in 2/3 of those cases, we can just do $this->instance->getField().

Status:Needs review» Reviewed & tested by the community

Thanks ! Looks good to me.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, field-ui-1946404-55.patch, failed testing.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new594 bytes
new105.73 KB
FAILED: [[SimpleTest]]: [MySQL] 55,497 pass(es), 17 fail(s), and 10 exception(s).
[ View ]

Grrr.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, field-ui-1946404-59.patch, failed testing.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new2.77 KB
new105.09 KB
PASSED: [[SimpleTest]]: [MySQL] 55,408 pass(es).
[ View ]

FieldInfo has ModuleHandler injected, so we cannot serialize that either.

This is #53 + the non-breaking fixes suggested in #54/#55

Title:Convert forms in field_ui.admin.inc to the new form interfaceChange notice: Convert forms in field_ui.admin.inc to the new form interface
Priority:Major» Critical
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

Committed 6ed57d3 and pushed to 8.x.

During the commit I fixed the following comment as we no longer use an access callback...

       // used for all bundles of a given entity type; but depending on
       // administrator settings, each bundle has a different set of view
       // modes available for customisation. So we define menu items for all
-      // view modes, and use an access callback to determine which ones are
+      // view modes, and use a route requirement to determine which ones are
       // actually visible for a given bundle.
       $items["$default_path/display/default"] = array(
         'title' => t('Default'),

Yay ! Thanks a ton @timplunkett !

Status:Active» Needs review
Issue tags:-Needs change record

Title:Change notice: Convert forms in field_ui.admin.inc to the new form interfaceConvert forms in field_ui.admin.inc to the new form interface
Priority:Critical» Normal
Status:Needs review» Active

Looks good to me.

Assigned:tim.plunkett» Unassigned
Status:Active» Fixed

Marking as fixed, then ?

Status:Fixed» Closed (fixed)

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

This FormInterface conversion brought up a very nasty side effect on PHP 5.4: #2025451: Drupal\entity_reference\Tests\EntityReferenceAdminTest fails on PHP 5.4.9 and libxml 2.9.0. Reviews appreciated :)