Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Debugging from #955926: Path not linked to node id after previewing then saving node turned up that when you preview a node that $node->uri['path'] gets set to 'node/' when the node eventually gets passed to node_save()/hook_node_insert(). This is because node_preview() operates directly on the $node reference object, which later gets saved to $form['#node'], and used to build the node object when the 'Save' button is pressed. So node previewing calls node_view(clone $node, 'teaser') which is great, but then just two lines down it calls node_view($node, 'full') - without a clone! And because template_preprocess_node() calls entity_uri() on the node object, but the node does not yet have an ID, we get $node->uri['path'] = 'node/'.
So this messes up any type of module that wants to also use entity_uri() on the $node object during hook_node_insert() or hook_entity_insert()!
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 1025870-node-preview-missing-clone.patch | 948 bytes | Dave Reid |
| #1 | 1025870-node-preview-missing-clone.patch | 702 bytes | Dave Reid |
Comments
Comment #1
Dave ReidComment #2
Dave ReidAdding pathauto tag as this has to be worked-around for now.
Comment #3
Dave ReidAdded docs. We don't need tests for this as it's obvious the second call to node_view() should have also been a clone, and it's nearly impossible to write a core test for this.
Comment #4
Dave ReidComment #6
robbycandra CreditAttribution: robbycandra commentedsubscribe
Comment #7
Dave Reid#3: 1025870-node-preview-missing-clone.patch queued for re-testing.
Comment #9
james.williams CreditAttribution: james.williams commentedsubscribe
Comment #10
lotyrin CreditAttribution: lotyrin commentedWe took this clone out in #844388: Taxonomy terms disappear from node preview if previewed more than once, which seems to have been reopened to consider returning it, with a patch to do so.
Comment #11
lotyrin CreditAttribution: lotyrin commentedI think cloning might be the wrong way to go about this. Preview setting the path seems like errant behavior that should simply be fixed. A previewed node doesn't have a path, so it shouldn't be set.
Comment #12
lotyrin CreditAttribution: lotyrin commentedThe way I interpret this issue is as follows:
node_uri makes an errant assumption that the called node will always have an nid.:
A previewed node doesn't have one, so this returns 'node/' which is invalid.
It seems that this should check for $node->nid, and return FALSE if it's not set.
Comment #13
lotyrin CreditAttribution: lotyrin commentedOw, there's a whole load of things piled on this assumption though.
Wherever $node->uri gets set doesn't check for an empty uri, even though entity_uri() can theoretically return one.
Also, if we allow uri callbacks to return empty values, entity_uri() needs to catch when that happens. Currently, it'll build an empty array and put 'options' in it, instead of itself returning an empty value.
template_preprocess_node() also assumes that entity_uri() always returns a path, blindly calling url() on the return value to set $node_url.
even node.tpl.php can't handle the case that $node_url is empty or not set, the <a> it creates to link to the url isn't conditional.
Cloning on preview fixes pathauto, and is probably the way to quickly get this done. I might open a 8.x issue for the above concern.
Comment #14
SilviaT CreditAttribution: SilviaT commentedsub
Comment #15
pillarsdotnet CreditAttribution: pillarsdotnet commentedBumping and tagging as per standard policy.
Comment #16
sunNot limited to nodes.
Seriously, after thinking through this and all the other related issues filed against core, the only proper solution is to add:
in node_view() and similar functions, and
in node_save() and similar functions.
Comment #17
webchickThis sounds like an edge case to me. Degrading to 'normal' priority. If this is worth blocking features on D8, please explain why.
Comment #18
attiks CreditAttribution: attiks commentedrelated bug report #1538796-3: file_unmanged_copy() fails with "The specified file could not be copied, because no file by that name exists" and #1540010: pathauto_entity_entity_insert causes file uploads to fail with File Entity
Comment #19
mgiffordComment #30
quietone CreditAttribution: quietone at PreviousNext commentedAs far as I can tell this is outdated due to #1510544: Allow to preview content in an actual live environment
Therefore, closing as outdated. If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.
Thanks!