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

["Chris Chabot (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SHINDIG-887?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Chabot resolved SHINDIG-887.
----------------------------------

       Resolution: Fixed
    Fix Version/s: 1.0.x-incubating
                   1.0.0-rc1
                   trunk
         Assignee: Chris Chabot

Awesome, patch looks great to me and does indeed our most famous todo gadget!

Thanks for the fix, patch's been committed

> processContent can get called twice with 'profile' as the view, causing the gadget to be rendered twice.
> --------------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-887
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-887
>             Project: Shindig
>          Issue Type: Bug
>          Components: Gadget Rendering Server (PHP)
>         Environment: Firefox 3.0.5 and Safari 3.2.1 on OS 10.5 PPC
>            Reporter: Joe Pletcher
>            Assignee: Chris Chabot
>             Fix For: trunk, 1.0.0-rc1, 1.0.x-incubating
>
>         Attachments: processContent_twice.patch
>
>
> 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/%3C170a26990901070205i59f416e3y4451462067461d7e@mail.gmail.com%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 == '') {
> +        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;

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.