Currently, entity_view() uses an heuristic based on the path to determine whether the entity is viewed in page mode or not. It matches the way Drupal core does it, which has already been determined as a bad idea (see #721754: A node cannot be displayed in different view mode on its own page).

I suggest we allow this mode to be set by the caller, or automatically determined by view mode. After all, most of the time this is what you want (I would argue that the 'full' mode for core nodes has very little use outside a full page context).

Files: 
CommentFileSizeAuthor
#9 entity_page_follow_up.patch569 bytesfago
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity_page_follow_up.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#6 1208864-page-mode-entity-rendering.patch5.37 KBDamien Tournoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-page-mode-entity-rendering.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#3 1208864-option-1.patch4.58 KBDamien Tournoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-1_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 1208864-option-1.patch4.58 KBDamien Tournoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-1.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 1208864-option-2.patch2.88 KBDamien Tournoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-2.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.88 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-2.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
new4.58 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-1.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

I see two options:

Option 1. Implement something close to what Drupal 6 was doing: add an explicit $page argument to entity_view(). The old behavior is kept when this argument is not present. We can even make this completely backward compatible if we don't change the EntityAPIControllerInterface interface (but I would recommend modifying it).

Option 2. Allow the view mode to explicitly say that they should be displayed in full page, by adding a new key to the view modes array.

I would recommend option 1. Option 2 seemed like a good idea when I wrote it, but I think leaving the caller decide is the best we can do.

Title:Per view mode 'page' settingMake the 'page' setting for entity view useful again

StatusFileSize
new4.58 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-option-1_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Reuploading option 1 to fix a typo.

Entity view returns the build array, so Option 2 still let the caller decide, but Option 1 seems less confusing. Maybe the two could be done and optional, one overriding the other.

Status:Needs review» Needs work

I agree that ideally the caller should be able to decide upon this.
With option2 one might end up with viewing in page-module if people re-use the view mode somewhere else, e.g. from a view. In most cases that's probably unwanted behaviour then, so let's require $page to be explicitly set (option1)

@patch:
* Problem is entity_view() also serves as harmonized entity_view() for core entities, where this option does not exist. So we'll need to document that this option only works for entities provided via the EntityAPIControllerInterface.
* Also, we need to document the default behaviour of guessing if NULL "is passed".
* It's a bit unfortunate to have another API change, but most people should use or extend the provided controller anyway - beside you.. ;) So I guess it should be ok.

For d8, I think we should migrate entity_view() to take a $options array like many other functions.

Status:Needs work» Needs review
StatusFileSize
new5.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1208864-page-mode-entity-rendering.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Rerolled, based on comments from #5.

Status:Needs review» Fixed

Thanks, committed.

-> We have an API Change for EntityAPIControllerInterface::view():

Modules that override the method of the default controller or implement the interface themselves have to follow the following change:

-  public function view($entities, $view_mode = 'full', $langcode = NULL) {
-    $view = parent::view($entities, $view_mode, $langcode);
+  public function view($entities, $view_mode = 'full', $langcode = NULL, $page = NULL) {
+    $view = parent::view($entities, $view_mode, $langcode, $page);

Status:Fixed» Closed (fixed)

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

Status:Closed (fixed)» Needs review
StatusFileSize
new569 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity_page_follow_up.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

AS #1210806: Add corollary to node_is_page() has shown, we forgot adding $page to Entity::view().

Doing so is an API change. However usually modules should override buildContent() not view(), so it should be ok.

Status:Needs review» Reviewed & tested by the community

Couldn't be more straightforward.

Yes, sorry about that.

Status:Reviewed & tested by the community» Fixed

Committed.

Status:Fixed» Closed (fixed)

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