Hello,

Does anyone happen to know what layout Inline Diff uses?

Each time I select an option from 'Highlight Changes' (Inline Diff), I experience a couple of quirks (I'm sure they're user based) :
- My Display Suite layout is overwritten. The left column is still there containing the title, body, author, but my right column goes away. The code no longer reflects the layout of display suite.
- Blocks I have disabled, appear; namely Print/Email/PDF and QuickTabs.

Any assistance would be appreciated. Thanks,
Christina

CommentFileSizeAuthor
#7 diff-display-suite-integration-1706466-7.patch955 byteskscheirer
#6 diff-display-suite-integration-1706466-6.patch1.01 KBkscheirer

Comments

alan d.’s picture

alan d. commented

Appears to be two issues here:

The AJAX request replaces the area with the #node-XX. Anything else in this area would get replaced. So things disappearing is probably by design

But, a real page submit could work ok, if the non-AJAX version worked. This is the first time I've tried (just started helping out), and I have discovered that this is broken, or I should say that the possibly complex logic in capturing the page output from the page that the block is on, then replacing the content with the inline diff has not be done. [i.e. what is does do is probably the best thing without investing too much time into it]

So sorry, no quick fixes :(

If you can, see if you can shift the HTML ID "node-NID" to contain just the main node content. If you can, then this should allow the AJAX to work independently of the rest of the page.

cbrasfield’s picture

cbrasfield commented
Category: bug » support

Thanks for your quick response Alan.

The display suite layout was in the main content portion of the markup. However, it appears that node_load doesn't account for alterations modules make to the structure of the content. node_view, however does.

I modified diff.pages.inc function diff_inline_show, changing functions node_load and node_build to node_view.

function diff_inline_show($node, $vid = 0, $metadata = TRUE) {
  $new_node = $vid ? node_load($node->nid, $vid, TRUE) : clone $node;
  $new = drupal_render(node_view($node, $view_mode = 'full', $langcode = NULL));

  $old = $vid ? _diff_get_previous_vid(node_revision_list($node), $vid) : 0;
  if ($old) {
    $old = drupal_render(node_view(node_load($node->nid, $old, TRUE), $view_mode = 'full', $langcode = NULL));
    $output = $metadata ? theme('diff_inline_metadata', array('node' => $new_node)) : '';
    $output .= diff_get_inline($old, $new);
    return $output;
  }
  return $new;
}

I haven't looked into the non-ajax version, but is there a reason the value of $node is different ($form['node']['#value'] vs $form_state['values']['node'])?

Thanks,
Christina

cbrasfield’s picture

cbrasfield commented
Category: support » bug

Changed category from support request to bug. Inline Diff doesn't appear to render the node in it's fully structured state when changing between different revisions.

crystal_alexandre_froger’s picture

crystal_alexandre_froger commented
Category: support » bug

+1 - It does not apply to Display Suite only: what about template files that explicitly output fields, for instance?
May not be a Drupal good practice, but still - diff not taking into account the template file the node is using does not seem being a good solution either.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Active » Needs work

so it seems like we need to make #2 into a patch and then get some reviews on it.

@cbrasfield - can you provide more detailed instructions on how to reproduce this bug using Display Suite?

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Needs work » Needs review
StatusFileSize
new diff-display-suite-integration-1706466-6.patch1.01 KB

#2 converted to patch, not tested.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
StatusFileSize
new diff-display-suite-integration-1706466-7.patch955 bytes

Removed the default arguments from the node_view() calls, those are not needed. Other this patch is the same as #2 and #6.

cyberschorsch’s picture

cyberschorsch
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

I can confirm that this patch is working.

alan d.’s picture

alan d. commented
Status: Reviewed & tested by the community » Needs work

This should use the clone to prevent a nasty potential recursive loop.

+  $new = drupal_render(node_view($new_node));
alan d.’s picture

alan d. commented

Marked #1320162: Inline diff not working when node is based on a Panel page as a duplicate as it suggests the same symptoms.

rdeboer’s picture

rdeboer
Location Melbourne
commented
Issue summary: View changes

I recently wrote an extension to the greate Diff module, which I called Diff Different: https://drupal.org/project/diff_different.
During that work I initially ran into problems with Display Suite too, but solved them.
Could be worth having a look at the code of Diff Different?

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Closed (outdated)

Given the proximity of EOL for Drupal 7, marking this as won't fix. The likely final Drupal 7 release of diff has landed in https://www.drupal.org/project/diff/releases/7.x-3.5. If this is something still applicable to a more modern version of the module, feel free to re-open and post additional notes.