Link attributes being double-serialized on node updates. [#1529256]

First of all, thanks for your work on the link field. We use it a lot and appreciate it.

We recently encountered the error described in this issue: http://drupal.org/node/268891
and applied this patch http://drupal.org/files/link-268891-3514936-2.patch

As mentioned in that issue (http://drupal.org/node/268891#comment-5638218), the patch is causing link attributes to be double-serialized on node updates. In that issue Digidog suggested a new issue be filed about it.

To reproduce:

Install Drupal 6.25, the latest -dev link, and the latest content module (CCK).
Update the page content type so that it has a link field, accept all defaults.
Create a new node, enter a title and a URL for the link.
Save it.
Edit the node.
Save it again.

The first time the node is saved, the attributes are saved in the database as: a:0:{}
The second time the node is edited and then saved, the attributes are stored in the database as: s:6:"a:0:{}";

I think it's because when a node is saved (not the first time, but when it's updated), both "presave" and "update" are called, so _link_process is called twice from link_field() and _link_process serializes the attributes.

It doesn't appear to cause any issues since the values are double unserialized if necessary (first by _link_load and then by _link_sanitize), but I'm not sure if it would affect other modules or other situations, so I thought I'd mention it.

When investigating this, I tried removing all calls to unserialize and serialize and using the attribute 'serialize' => TRUE for the attributes database column in link_field_settings(). I think that means the content module handles the serializing for you. It seems to save the attributes in serialized format and read them from the database OK, but I have not tested in depth. I can send a patch if you'd like.

   case 'database columns':
      return array(
        'url' => array('type' => 'varchar', 'length' => LINK_URL_MAX_LENGTH, 'not null' => FALSE, 'sortable' => TRUE),
        'title' => array('type' => 'varchar', 'length' => 255, 'not null' => FALSE, 'sortable' => TRUE),
        'attributes' => array('type' => 'text', 'size' => 'medium', 'not null' => FALSE, 'serialize' => TRUE),
      );

Comment	File	Size	Author
#3	link-serialization_issue-1529256.patch	474 bytes	T2k
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

dboulet CreditAttribution: dboulet commented 4 March 2014 at 06:32

Priority:

Minor

» Major

This is definitely something that should be fixed. In some cases, for instance when displaying links in views, _link_load() is not called and the link attributes are not properly unserialized.

For example, see: #1578340: link title not displayed using views

Comment #2

Hardcode CreditAttribution: Hardcode commented 18 July 2014 at 22:30

Version:

6.x-2.x-dev

» 6.x-2.11

This problem still exists.
It happens when a new node revision is made. I fixed it on my server by removing case 'update':
in function link_field.
This should definitely be corrected.

Comment #3

T2k CreditAttribution: T2k commented 3 October 2014 at 14:19

File	Size
link-serialization_issue-1529256.patch	474 bytes

A small patch related to this issue.

Comment #4

hermes_costell CreditAttribution: hermes_costell commented 13 October 2015 at 18:41

I encountered the same issue, thanks for the work done above to bring it to light. My solution was a bit different though. Unless I'm misunderstanding just commenting out the

case 'update':

Means that you can never edit a link again, right?

Instead I modified the code thusly:

foreach ($items as $delta => $value) {
    if(!empty($items[$delta]['attributes'])){
      if (!is_array($items[$delta]['attributes'])) {
         $items[$delta]['attributes'] = (array)unserialize($items[$delta]['attributes']);
       }
    }
       
     _link_process($items[$delta], $delta, $field, $node);
}

link/link.module line 235

Unfortunately this means you have to visit each affected node with a mangled link and click "save" to apply the above fix, but it's a start.