Development

#9080 (sfWebDebug::injectToolbar can insert multiple sets of code (includes proposed solution))

You must first sign up to be able to contribute.

Ticket #9080 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

sfWebDebug::injectToolbar can insert multiple sets of code (includes proposed solution)

Reported by: caponica Assigned to: fabien
Priority: minor Milestone: 1.4.7
Component: other Version: 1.4.6
Keywords: sfWebDebug injection head body Cc:
Qualification: Unreviewed

Description

If you have a page with more than one </head> or </body> then sfWebDebug::injectToolbar inserts its styles and scripts next to every such instance.

This is not the desired behaviour - e.g. if you have a page which contains a text editor for editing some raw HTML then the </head> and </body> of the code you are editing gets the toolbar injected into it.

A simple solution would be just to replace the FIRST instance of </head> found on the page and just the LAST instance of </body>.

Changing this method to the following achieves this:

  /**
   * Injects the web debug toolbar into a given HTML string.
   *
   * @param string $content The HTML content
   *
   * @return string The content with the web debug toolbar injected
   */
  public function injectToolbar($content)
  {
    $position_of_first_head = strpos($content, '</head>');
    if ($position_of_first_head !== false) {
      $content = substr_replace($content, '<style type="text/css"><bobo />'.str_replace(array("\r", "\n"), ' ', $this->getStylesheet()).'</style></head>', $position_of_first_head, 7);
    }

    $debug = $this->asHtml();
    $count = 0;
    $position_of_last_body = strrpos($content, '</body>');
    if ($position_of_last_body !== false) {
      $content = substr_replace($content, '<script type="text/javascript">'.$this->getJavascript().'</script>'.$debug.'</body>', $position_of_last_body, 7);
    } else { // is this catch-all needed? See note below.
      $content .= $debug;
    }

    return $content;
  }

However (and this is a separate point), I would question why the original method always inserts the $debug at the end of content (even if it cannot find "</body>"). Wouldn't it be better to not inject the debug toolbar in these instances? Without the styles and scripts it makes little sense to insert the toolbar and removing this catch-all would stop the toolbar appearing when the parent action called something like

$this->setLayout(false);

which is a source of confusion for many people yet provides little benefit.

If you can think of scenarios where the relevant </head> and </body> are not the first and last ones then feel free to update this ticket, but I think that only changing one of each is going to be better than inserting multiple sets of debug toolbars, scripts and styles in the vast majority of cases!

Christian Caponica Ltd

Change History

(follow-up: ↓ 2 ) 09/22/10 04:17:27 changed by Kris.Wallsmith

  • status changed from new to closed.
  • resolution set to fixed.

(In [30951]) [1.3, 1.4] fixed WDT injects multiple times (closes #9080)

(in reply to: ↑ 1 ) 09/22/10 09:45:52 changed by ahto

Replying to Kris.Wallsmith:

(In [30951]) [1.3, 1.4] fixed WDT injects multiple times (closes #9080)

The debug bar doesn't work after this change. This part was left out '<script type="text/javascript">'.$this->getJavascript().'</script>'

09/22/10 11:36:43 changed by Kris.Wallsmith

(In [30961]) [1.3, 1.4] fixed missing WDT javascript (closes #9083, refs #9080)

09/22/10 11:43:24 changed by caponica

Kris, thanks for looking at this, cleaning up my code and updating the code base so quickly!

C