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.
The title is not passed to hook_field_diff_view if the "No title" option is chosen in the field settings and a php warning is raised.
Patch attached.
Comment | File | Size | Author |
---|---|---|---|
#11 | 2480723-undefined-index-title-10.patch | 574 bytes | naidim |
#1 | no-title-warning-2480723-0.patch | 562 bytes | krystalcode |
Comments
Comment #1
krystalcode CreditAttribution: krystalcode commentedComment #2
sumitmadan CreditAttribution: sumitmadan commentedI am not able to reproduce the issue. Can you please provide more information on this (which version of diff you are using etc.)?
Comment #3
rrrob CreditAttribution: rrrob commentedConfirmed patch in #1 resolves this error.
Using diff version 7.x-3.2
Comment #4
rrrob CreditAttribution: rrrob commentedComment #5
sumitmadan CreditAttribution: sumitmadan commentedStill I am not getting the error. Please let me know how and where I can see the error?
Comment #6
Argus CreditAttribution: Argus commentedI cannot reproduce this on a regular Drupal install, but I can reproduce it on an Open Atrium installation.
Note that Open Atrium has a submodule oa_diff that allows a specific revision to be indicated. Perhaps this is causing the error?
Created an issue in the OA issue queue that links to this one. #2748703: Setting Link title to "no title" produces a PHP Notice
Comment #7
krystalcode CreditAttribution: krystalcode commentedI was working with a normal Drupal installation when I filed this bug - it's been long time and I don't have the code so I can't provide more details at the moment. It's most likely related to PHP version though.
$diff_items[$delta] = $item['title'] . $item['url'];
It tries to render $item['title'] while it doesn't exist, and certain versions of PHP may raise warnings. The patch sets this value to an empty string, if it is not already set. It is straight forward and it won't introduce any regression, so I would suggest that it is merged.
Comment #8
dqd@krystalcode: while I would agree we still need input here. Setting it to an empty string can cause problems on other spots inside the diff functionality part (not sure) where it is required that it doesn't exist if not set. Title is an option and therefore we maybe should rather use an if isset, take it in, if not, than do-whatever without it than setting it to an empty existing string. Especially in user interaction it can confuse to see an empty title somewhere even if title is not set by content admin.
I've tried another way to wrap isset() around before the main action, but with no luck (WSOD on diff page). Diff still requires both fields which link supports, even if not set.
That was the last minor time I had today to spend for this, sorry ... thoughts ?
Comment #9
krystalcode CreditAttribution: krystalcode as a volunteer commentedHi @diqidoq,
I originally resorted to setting it to empty string in order to keep the code cleaner by avoiding nested statements. I understand the concern and I would be happy with either approach.
However notice that the arguments are not passed by reference and setting the $item['title'] to empty string here won't affect other parts of the program. The function is short and it is simply preparing a string and returning it, so there is little chance of confusion. In that context, I would personally vote for readability over semantics, but I'd be happy with your approach as well.
Yeah agreed, this is of minor priority - although it is a warning and not notice and I would still consider it a bug.
Comment #10
naidim CreditAttribution: naidim as a volunteer commentedTo Duplicate the error: Install Link, Diff modules. Create a Content Type that has a Link field without a Title. Create then Edit a node of that Content Type. Click "View Changes" button.
Notice: Undefined index: title in link_field_diff_view() (line 14 of /sites/uvm/content/sites/all/modules/contrib/link/link.diff.inc).
Notice: Undefined index: title in link_field_diff_view() (line 18 of /sites/uvm/content/sites/all/modules/contrib/link/link.diff.inc).
What I believe is the issue: link.diff.inc line 14 just checks $item['title'] without checking if it exists (undefined index) and line 18 tries to concatenate $item['title'] even if it is not set or doesn't have a value.
Fix: Instead of setting it to empty string if not set, just check if isset, and don't concatenate non-existing/empty title.
line 14: if ($item['url'] && isset($item['title'])) {
line 18: $diff_items[$delta] = $item['url'];
Comment #11
naidim CreditAttribution: naidim as a volunteer commentedPatch with previously mentioned code changes. Tested with Title links and no title links and appears to work as intended.
Comment #12
jbitdrop CreditAttribution: jbitdrop commented@crystalcode @diqidoq @naidim Thanks for all the input on this issue. Helped a lot to tackle down the issue. Will review the latest patch asap.
Comment #13
renatogHi guys.
Thank you very for contributions.
I applied the patch #11 and works good for me.
Regards.
Comment #15
renatogFixed.
Committed to the dev branch.
Thank you very much.
Regards.