Popups in Drupal 7: Plugable renderers for generating content

I don't like the "unthemed" things, and it doesn't make sense either.. If it is 'unthemed', why is there a page-unthemed.tpl.php after all?

Can't you simply rename page-unthemed.tpl.php to modal.tpl.php? (Makes more sense imo)
And then, you can probably make $unthemed => $modal...

I'm not sure, but $unthemed just doesn't feel right.. It really has to change into something else, question is what...

Updated and freshly re-rolled a patch.
- made untheme-page => modal-page.tpl.php;
- variables are not called 'unthemed' anymore, but 'modal' (in this time on the web, everybody knows what a 'modal' is, and what it's does. So I think this is okay for now);

This would really get us started on things...

Though, I think we should rename this issue to something like: 'Modal support for drupal 7'

Is this related in any way to #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.). I would think this would be just another format to render into and that's exactly what that issue is about.

subscribing

Subscribing.

Subscribing.

this looks like a hack to me, to be honest. i will think about alternative solutions ... fyi, there are still two instances of $unthemed in the patch.

I agree with Moshe. I'd rather have a nice REST interface which returns things in some XML format (I suppose there is a module for something like this? :P). Parsing XML using jQuery isn't much harder anyway ;)

Edited - Whoops, wrong issue.

This looks like a good approach. Certainly we will need a hook_renderers (or something like it), and the code to detect what renderer to use. It will need some refactoring if my plan for the page as an array stuff gets in, but I think it is a nice step in the right direction (and lets people work on the frontend stuff in the meantime at the very least). The drupal_goto avoidance sounds like a good idea too, this is often a pain - I haven't looked at the implementation in detail though.

Subscribing. This sounds very exciting, but as starbow mentions it touches on a lot of key areas so we need to make sure we get it "right". (I consider "right" being turning Drupal from a CMS into a web services server, of which an XHTML page is just the default web service. :-) )

Subscribing.

subing

Nice work starbow, thanks for pushing this priority area forward.

This patch has the benefit of simplicity. It's not a full solution, but it's a useful concrete sketch of some first steps. Is this the way to go? I'm not sure, but I think it warrants further exploration and work.

The latest patch has good improvements in terms of moving from a popups specific approach to a flexible, general multiple rendering solution, though more work is needed.

Specific code comments:

1. Whether or not we're rendering into a popup isn't the main concern. We'll want a general JSON method. So the key here probably should be javascript/json rather than json/popup.

+function system_renderers() {
+  return array(
+    'xhtml/plain' => 'system_render_default',
+    'json/popup' => 'system_render_json_popup'
+  );
+}

2. system_render_json_popup()

Here's the catch. So far, this function supports a single hook, 'page', rejecting all others. What if we want to extend or modify what's returned with a JSON 'page'? What if we want to return e.g. a node? We'll end up needing something equivalent to what we have with the HTML renderer (the theme system).

Possible approaches:

* pass all JSON data through a drupal_alter() call, enabling modification of the data before return. See this issue: http://drupal.org/node/241450.

* non-page data are returned as follows:

    default: 
      return drupal_json(array(
        'status' => TRUE,
        'hook' => $hook,
        'content' => $args[0],
      ));

And minor issues: 'status' should be boolean, drupal_get_title() should be filtered.

3. Error handling in index.php

Currently the patch handles only success in index.php. MENU_NOT_FOUND, MENU_ACCESS_DENIED, etc. will be returned as HTML. We need handling for these cases. See relevant code in my patch here: http://drupal.org/node/145551#comment-558186.

4. system_after_process_form() isn't really a hook implementation, since only one implementation can be called. Maybe it's just a function call?

5. In system_after_process_form() we shouldn't have to resort to calling exit to override further processing.

6. Request and render modes.

+  $render_mode = isset($_SERVER['HTTP_X_DRUPAL_RENDER_MODE']) ? $_SERVER['HTTP_X_DRUPAL_RENDER_MODE'] : 'xhtml/plain';
+  $request_mode = isset($_SERVER['HTTP_X_REQUESTED_WITH']) ? $_SERVER['HTTP_X_REQUESTED_WITH'] : 'standard';  

As discussed in http://drupal.org/node/145551, a header is not always appropriate o determine the specific expected return format. E.g., jQuery uses the same value for $_SERVER['HTTP_X_REQUESTED_WITH'] whatever the data format being used (XML, JSON, etc.). In other cases, e.g., when providing RSS feeds, we can't expect a specific header--the address had to be enough. How can we flexibly determine the render format?

It's not clear why we need separate request and render modes. in system_after_process_form() (the only place $request_mode is used), it looks like the $render_mode could be used instead. We're evaluating the variable to determine if we should render in JSON.

More general questions:

7. The patch seems to do three things: refactors rendering, introduces a special case for JSON form submission handling, and does some special things for popups. Do these all belong in the same patch?

8. We have various other cases where what we want to render and return is JSON. Can we convert all drupal_json() calls to use this new rendering approach?

9. We have other rendering formats in core: RSS, XMLRPC. How would they be converted to renderers?

This is great work Tao. I'm really diggin' the approach you used with creating special Drupal HTTP request headers rather than using the $_SERVER['HTTP_ACCEPT']. I think it's a much more flexible approach and much less likely to cause side-effects. We should look into how well other platforms besides JavaScript can set request headers like that (for example, we want to be sure that ActionScript can do the same thing from Flash if we were requesting XML or AMF data).

@nedjo, regarding concerns 1-3: We're specifically not creating a general JSON solution. The only scenario we're solving in core is for popups, which only needs a special rendering mode for the page itself. However, this patch should enable other modules to come in and create any kind of rendering mode they like, such as a complete XML, JSON, or other renderer. Just by setting special request headers, a module could display the entire site in a completely different format. It's likely that JSON and XML modules will belong in contrib, making it so that you could visit node/1 and get the node object in XML or JSON. This functionality won't be in core, but the framework should be there to support it.

Which brings me to a slightly different approach to this patch. The current approach in #20 only uses the render() function in one place: page.tpl.php. As we want more places to have multiple rendering modes, we'd be making the same change of theme() to render() calls everywhere we need multiple modes. Ultimately, this would result in theme() being replaced with render() because we'll want to have multiple rendering modes for nearly all output.

With that in mind, we can save a lot of work by making theme() into a multi-format output function. This patch makes it so that it no longer necessarily invokes the theme system and loads templates, instead that's just the default behavior. Here's the central part of the functionality:

The new theme() function:

function theme() {
  global $render_mode;
  $args = func_get_args();

  // Check if we're using a custom render mode.
  if ($render_mode != 'xhtml/plain') {
    static $theme_renderers;
    if (!isset($theme_renderers)) {
      $theme_renderers = module_invoke_all('theme_modes');
    }
    $theme_renderer = isset($theme_renderers[$render_mode]) ? $theme_renderers[$render_mode] : NULL;

    // If a different theme rendering system is available, call it instead of the default.
    if (function_exists($theme_renderer)) {
      return call_user_func_array($theme_renderer, $args);  
    }
  }

  // If not using a custom render mode, use the default renderer.
  return call_user_func_array('theme_render', $args);
}

Basically, the old theme function is renamed to theme_render() and is the default behavior for rendering. This means that any function that runs through theme() can now have multiple rendering modes. System.module implements a render mode specifically for popups, which outputs JSON for theme_page(), but just calls the normal theme function for all other renderings.

To match the new terminology, hook_renderers has been renamed to hook_theme_modes, since they're now providing different modes of rendering items passed through the theme() function.

This approach makes Drupal the ultimate outputting machine. It makes it so that a module could implement an entire rendering system equivalent to the core theme system, but output in an entirely different format. Since all Drupal output is run through a theme() function at some point, this means that all output could have multiple ways of being displayed. It also ultimately results in almost no changes to existing code to allow multiple modes, since everything output through theme() will be given this flexibility.

That said, this is a really major change to the way Drupal outputs. Feel free to criticize the approach, let's just make sure we get it right. :D

I re-rolled the Popups patch in 193311#69 which you can use to test the #26 patch above.

More replies to nedjo:

4. system_after_process_form() isn't really a hook implementation, since only one implementation can be called. Maybe it's just a function call?

I can see where this needs to be a hook. If other modules provide different rendering modes, they might need to catch the processing of the form and return data back the browser in a different format also, just like popups require a JSON return instead of HTML.

5. In system_after_process_form() we shouldn't have to resort to calling exit to override further processing.

See responses to 8 and 9 below.

6. Request and render modes.

As discussed in http://drupal.org/node/145551, a header is not always appropriate o determine the specific expected return format. E.g., jQuery uses the same value for $_SERVER['HTTP_X_REQUESTED_WITH'] whatever the data format being used (XML, JSON, etc.). In other cases, e.g., when providing RSS feeds, we can't expect a specific header--the address had to be enough. How can we flexibly determine the render format?

It's not clear why we need separate request and render modes. in system_after_process_form() (the only place $request_mode is used), it looks like the $render_mode could be used instead. We're evaluating the variable to determine if we should render in JSON.

I agree. I don't see the purpose of the $request_mode global variable when it looks like $render_mode would suffice just fine. It's also strange that the in system_after_process_form() the $request_mode is 'XMLHttpRequest' while the returned data is JSON. Doesn't seem to be natural to me.

8. We have various other cases where what we want to render and return is JSON. Can we convert all drupal_json() calls to use this new rendering approach?
9. We have other rendering formats in core: RSS, XMLRPC. How would they be converted to renderers?

Contrary to what I said above (We're specifically not creating a general JSON solution.), maybe we should provide this so that we could eliminate the preemptive exiting of the page. Imagine a single menu callback that sometimes needed to return JSON and other times HTML. Both could use the single theme() function call like this:
return theme('mymodule_callback');

Then using the swappable theming modes provided by #26, we could make two different theme functions that would output depending on the $render_mode:

// Normal HTML output:
function theme_mymodule_callback() {
  return '<div>new content</div>';
}

// text/javascript $render_mode output:
function json_mymodule_callback() {
  return array('content' => 'new content');
}

The system renderer for JSON output then would not output anything to the page for anything run through theme() unless there was an explicit $hook callback with the json_ prefix, like the above example.

subscribing

I spoke with quicksketch in IRC earlier today about this. Here's my commentary, subject to additional conversation. :-)

This is exciting!!! :-) Giving Drupal multi-output support is an important step toward making Drupal a general purpose web services server (where HTML is just another service). Count me a huge +1 on the concept.

Regarding the implementation, the mime-type of the request is a good item to "detect" on. However, if we're going to use our own custom mime-types then is there a standard for "non-standard" mime-types the way there is for HTTP or SMTP headers? I have seen x-foo/bar style mime-types before, I think, although I'm not sure if that's a de jure standard or just a convention perpetuated by Microsoft. :-) If so, then any non-standard headers we use should use a "proper" custom format. (x-drupal/popup, or whatever).

That said, I am not convinced that we need to introduce custom mime-types. In particular, I am convinced that we do *not* need to use the word "popup". From Drupal's perspective, it doesn't care if an HTML fragment it returns is used in a pop-up, in a pop-in (expand into a div in the page), is used to open a new browser window, is part of a form AHAH request, or whatever. It cares that it's an HTML fragment, returned via a JSON envelope, or an HTML fragment returned "naked", etc. So the request type shouldn't be "send me a popup", it should be "send me an HTML fragment in a JSON envelope", "send me a node in JSON format", "send me a user as a SOAP object", etc. with a default of "send me a full HTML page with page theming" (as opposed to a full HTML page that does not have the blocks and such around it, for use in an iframe.)

That leads into my next point; I'd think this system would have to integrate into the menu system. chx's "Drupal Pipes" brain dump from a few weeks ago is one approach, but not necessarily the best. If I can think aloud here a bit:

function node_menu() {

  $items['node/%node']['text/html'] = array(
    'access callback' => 'access content',
    'response callback' => 'node_page',
    //...
  );
  $items['node/%node']['text/html-fragment'] = array(
    'access callback' => 'access content',
    'response callback' => 'node_page',
    //...
  );
  $items['node/%node']['application/json'] = array(
    'access callback' => 'access json content', // Maybe we don't need this to be different, I don't know.
    'response callback' => 'node_load',
    //...
  );
  $items['node/%node/edit'] = array( // This handler only has the default theme mode and nothing else.
    'access callback' => 'edit foo content'
    'response callback' => 'node_edit',
    //...
  );
  //...
}


function system_theme_modes() {
  $modes['text/html'] = array(
    '200' => array(
      'response type' => 'text/html',
    ),
    '403' => array(
      'handler' => 'drupal_denied_page',
      'response type' => 'text/html',
    ),
    '404' => array(
      'handler' => 'drupal_not_found_page',
      'response type' => 'text/html',
    ),
  );
  $modes['text/html-fragment'] = array(
    '200' => array(
      'handler' => 'theme_fragment',
      'response type' => 'text/html',
    ),
    '403' => array(
      'handler' => 'drupal_denied_fragment',
      'response type' => 'text/html',
    ),
    '404' => array(
      'handler' => 'drupal_not_found_ahah',
      'response type' => 'text/html',
    ),
  );
  $modes['application/json'] = array(
    '200' => array(
      'handler' => 'json_encode',
      'response type' => 'application/json',
    ),
    '403' => array(
      'handler' => 'drupal_denied_json',
      'response type' => 'application/json',
    ),
    '404' => array(
      'handler' => 'drupal_not_found_json',
      'response type' => 'application/json',
    ),
  );
  return $modes;
}

function foo_theme_modes_alter() { /* Just because. :-) */ }


function theme_page() {
 // same as now.
}

function theme_ahah($output) {
  return $output;
}

function drupal_denied_page() { /* ... */ }

Then the logic for menu_execute_active_item() would look something like the following pseudocode:

- Pick out our request mime-type from the HTTP headers.
- Load the active menu item.
- If no active item found:
-- Call the 404 handler for the specified mime-type.
-- Set the HTTP response mime type.
-- Print the result.
-- Return
- Pick out the sub-item for the mime type we were asked for; if none specified, default to text/html.
- Set the HTTP response mime type.
- If the user does not have the right access permissions:
-- Call the 404 handler for the specified mime-type.
-- Print the result.
-- Return
- If the user has access perms:
-- call the response callback.
-- If there is a handler for that response type:
--- pass the callback return through the handler (theme function).
-- Print the result.
- Else

Extensibility options this way:

- Dead-simple to add more handlers for different mime-types as necessary.
- Easy to add custom handlers for different response codes, say if we wanted to send something different on a 301 or 302 response.
- Between hook_menu_alter() and hook_theme_mode_alter(), a contrib can basically rewrite the entire controller/router mechanism entirely in a supported and easy-to-read fashion.
- Lots of other contrib-based extensibility options that I haven't even thought of.
- The mime-type of the request and response is, technically, just a dumb string so we can do all sorts of good (or stupid?) things with it.

Please someone tell me why the above is a stupid idea, because I'm sure I've overlooked something. :-)

Hm. I may have misunderstood your earlier post, since the string format you are using looks like a mime-type and there is an HTTP request header for specifying a mime-type. If that's not what you want to use, I'm cool with that, too.

Actually, that would offer a very nice way of specifying the "envelope". page/html would set the theme system to html-mode, and render the result of menu_execute_active_handler() as an HTML page. raw/html would set the theme system to html-mode, and simply return $content without additional processing (useful for popups and for pop-ins). json/html would set the theme system to html-mode, and return it is a json envelope. In the first 2 cases, a 403 or 404 response would return an HTML page or fragment with the appropriate HTTP status code. In the third, it would return an empty json object with the appropriate HTTP status code.

Which brings up another issue: We're not tying ourselves to only Drupal being able to use this system, so a 3rd party system that wants SOAP, RSS, or JSON data out of Drupal cannot make a request without a proprietary header. No RSS reader is going to do that.

We're not tying ourselves to only Drupal being able to use this system, so a 3rd party system that wants SOAP, RSS, or JSON data out of Drupal cannot make a request without a proprietary header.

I think the custom header is a convenient option, but fortunately it's not the only method we could support. Crell has mentioned to me that he opposes the idea of a global for $render_mode, but merlin suggested that drupal_get/set_render_mode() functions might be acceptable. If this were the case, either Drupal core or a contrib module could switch the rendering mode via another mean (a specific menu callback, a URL prefix, browser/RSS reader signature, whatever). So I don't think we're cornered into using the header method, but it does seem to be a very good option for doing things like the popups or AJAX form submissions.

Subscribing. This is good stuff.

Hah, starbow just pointed be to here from http://drupal.org/node/145551#comment-836572 where a considerably similar approach is suggested.

Subscribing to keep track of this.

I reviewed the latest patch, and I would not say that having Drupal specific HTTP headers is a good idea in my opinion. We strive to be a data hub for different peers, not necessarily Drupal jQuery code, but other data consumers as well. For them to be easy to integrate with Drupal, we need to look for standard ways of communicating what type of output to generate. I share Henrique's view at http://drupal.org/node/145551#comment-837326 that we should instead get the Accept header and other standard things going.

Comments by quicksketch at http://drupal.org/node/218830#comment-791590 above seem to be contradicting on this. First, he says the purpose of this patch is to enable popups and not to be a generic content machine, but then he says his patch makes Drupal the ultimate outputting machine. Now, what's the focus here?

To clarify:

We're specifically not creating a general JSON solution. The only scenario we're solving in core is for popups, which only needs a special rendering mode for the page itself. However, this patch should enable other modules to come in and create any kind of rendering mode they like, such as a complete XML, JSON, or other renderer.

I'm saying this particular patch is not a complete JSON outputting solution. It is an enabler of a complete JSON outputting solution. This patch provides flexibility, but the included functionality is only an implementation for popups.

subscribing

subscribing.

After reviewing #145551: Enable loading and rendering into multiple formats (html, xml, json, atom, etc.), which has more momentum, it looks like it'll provide the exact same features as this patch, only slightly different in terminology and mechanism. I'm marking this patch as duplicate. We can reroll the popups patch to work with that approach instead.

Subscribing...

Is this actually going into core?

Subscribing to come back and read the discussion.

It seems like this thread has moved over here, where I uploaded a new revised patch: #374646: Popbox (Popups Lite): Adding Modal Dialogs to Confirmations.

Hi there, I'm a long time reader first time poster...

I have been searching the forums trying to wrap my head around a way to implement a simple popup in Drupal 7. I am yet to find a clear solution. I hope you can point me in the right direction.

I have a client who wants to emulate a flash website. So the requirement is to have a bunch of links which each open a popup containing a title and some content (I assume a node). Possibly with a scroller and some sort of fade in / out effect etc.

All of the available modules are either directed at managing images, or admin tasks, or haven't been ported to Drupal 7.

Any help or ideas would be most appreciated!

Congratulations on your first post!

Could you please open this as a new support issue - it is bad form (because it is confusing for others) to mix support issues with new feature or bug issues.