[SHINDIG-887] processContent can get called twice with 'profile' as the view, causing the gadget to be rendered twice.

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.0, 1.1-BETA1
• Component/s: PHP
• Environment: Firefox 3.0.5 and Safari 3.2.1 on OS 10.5 PPC
• Assignee: Chris Chabot
• Reporter: Joe Pletcher
• Created: 24/Jan/09 12:24
• Updated: 19/Jun/09 12:30
• Resolved: 25/Jan/09 23:32

Description

I had a problem with the 'Todo' example not rendering properly (it was rendering twice, kinda). I found a thread from earlier this month where someone had the same problem. Ben Bonfil explained the root of the problem in an email here:

http://mail-archives.apache.org/mod_mbox/incubator-shindig-dev/200901.mbox/[e-mail]%3E

but unfortunately his solution did not work. Working through the code shows that some gadgets (in this case ToDo) indeed have two gadgets contained inside them, however processContent is run over both content elements, and will attempt to add both of these to the output. The solution would appear to be to loop through content twice and check the first time if a profile element is explicitly defined, and if this is the case ignore any content elements who do not have a view explicitly defined (i.e. content elements that would have been assigned DEFAULT_VIEW.) The second time would then run processContent, conditionally.

Ive included a patch which does this, and fixes it for me. I don't see a file attachment option, so I'm including it inline.

Thanks, if there's is anything else I can do, please let me know. Also, I'm unsure on the mailing list etiquette - should I reply to that thread? start a new one? email off list?

Thanks,
Joe

Index: GadgetSpecParser.php
===================================================================
— GadgetSpecParser.php (revision 736574)
+++ GadgetSpecParser.php (working copy)
@@ -54,9 +54,36 @@
foreach ($doc->UserPref as $pref)

{ $this->processUserPref($gadget, $pref); }

+ // Assume gadget v1
+ $explicit_profile = false;
foreach ($doc->Content as $content) {

• $this->processContent($gadget, $content);
  + $attributes = $content->attributes();
  + if (empty($attributes['type'])) { + throw new SpecParserException("No content type specified!"); + }
  + $view = isset($attributes['view']) ? trim($attributes['view']) : '';
  + // Note if we find a profile explicity defined
  + if (strpos($view, "profile") !== false) { + $explicit_profile = true; + }
  }
  + foreach ($doc->Content as $content) {
  + $attributes = $content->attributes();
  + if (empty($attributes['type'])) {+ throw new SpecParserException("No content type specified!");+ }

  + $view = isset($attributes['view']) ? trim($attributes['view']) : '';
  + // If view isnt defined and we didnt find a profile explicity defined
  + if ($view == '')

  Unknown macro: {+ if (! $explicit_profile) { + $this->processContent($gadget, $content, array(0 => DEFAULT_VIEW)); + }+ // If view isnt defined and a profile was found, this will catch it+ }

  else

  { + $views = explode(',', $view); + $this->processContent($gadget, $content, $views); + }

  + }
  foreach ($doc->ModulePrefs->Preload as $feature)

  { $gadget->preloads[] = new Preload($feature); }

  @@ -166,18 +193,9 @@
  $gadget->userPrefs[] = $preference;
  }

• private function processContent(&$gadget, $content) {
• $attributes = $content->attributes();
• if (empty($attributes['type'])) { - throw new SpecParserException("No content type specified!"); - }
• $view = isset($attributes['view']) ? trim($attributes['view']) : '';
• $views = explode(',', $view);
  + private function processContent(&$gadget, $content, $views) {
  $html = (string)$content; // no trim here since empty lines can have structural meaning, so typecast to string instead
  foreach ($views as $view) {
• if (empty($view)) { - $view = DEFAULT_VIEW; - }

  $viewSpec = new ViewSpec($view, $content);
  if (! isset($gadget->views[$view])) {
  $viewSpec->content = $html;