Replace onclick inline event handlers for createPopup (#2410)

* Move <script> before </body>

* Change makePopupLink to not use onclick

* Change makePopupButton to not use onclick

* Use .popup-link in control_functions.php

* Use makePopupButton in controlcaps.php

* Prevent double-encoding in makePopup*

* Use makePopupButton in devices.php

* Use makePopupButton in logout.php

* Use makePopupLink in monitor.php

* Use makePopupLink and .popup-link in montage.php

* Use makePopupButton in options.php

* Use makePopupButton, makePopupLink, and .popup-link in zones.php

web/includes/functions.php

@@ -400,14 +400,19 @@ function makeLink( $url, $label, $condition=1, $options='' ) {
 }
 
 function makePopupLink( $url, $winName, $winSize, $label, $condition=1, $options='' ) {
-  $string = '';
+  // Avoid double-encoding since some consumers incorrectly pass a pre-escaped URL.
+  $string = '<a class="popup-link" href="' . htmlspecialchars($url, ENT_COMPAT | ENT_HTML401, ini_get("default_charset"), false) . '"';
+  $string .= ' data-window-name="' . htmlspecialchars($winName) . '"';
   if ( $condition ) {
-    if ( is_array( $winSize ) )
+    if ( is_array( $winSize ) ) {
-      $popupParms = "'".$url."', '".$winName."', '".$winSize[0]."', ".$winSize[1].", ".$winSize[2];
+      $string .= ' data-window-tag="' . htmlspecialchars($winSize[0]) . '"';
-    else
+      $string .= ' data-window-width="' . htmlspecialchars($winSize[1]) . '"';
-      $popupParms = "'".$url."', '".$winName."', '".$winSize."'";
+      $string .= ' data-window-height="' . htmlspecialchars($winSize[2]) . '"';
+    } else {
+      $string .= ' data-window-tag="' . htmlspecialchars($winSize) . '"';
+    }
 
-    $string .= '<a href="'.$url.'" onclick="createPopup( '.$popupParms.' ); return( false );"'.($options?(' '.$options):'').'>';
+    $string .= ($options ? (' ' . $options ) : '') . '>';
   } else {
     $string .= '<a>';
   }
@@ -417,11 +422,20 @@ function makePopupLink( $url, $winName, $winSize, $label, $condition=1, $options
 }
 
 function makePopupButton( $url, $winName, $winSize, $buttonValue, $condition=1, $options='' ) {
-  if ( is_array( $winSize ) )
+  $string = '<input type="button" class="popup-link" value="' . htmlspecialchars($buttonValue) . '"';
-    $popupParms = "'".$url."', '".$winName."', '".$winSize[0]."', ".$winSize[1].", ".$winSize[2];
+  $string .= ' data-url="' . htmlspecialchars($url, ENT_COMPAT | ENT_HTML401, ini_get("default_charset"), false) . '"';
-  else
+  $string .= ' data-window-name="' . htmlspecialchars($winName) . '"';
-    $popupParms = "'".$url."', '".$winName."', '".$winSize."'";
+    if ( is_array( $winSize ) ) {
-  $string = '<input type="button" value="'.$buttonValue.'" onclick="createPopup( '.$popupParms.' ); return( false );"'.($condition?'':' disabled="disabled"').($options?(' '.$options):'').'/>';
+      $string .= ' data-window-tag="' . htmlspecialchars($winSize[0]) . '"';
+      $string .= ' data-window-width="' . htmlspecialchars($winSize[1]) . '"';
+      $string .= ' data-window-height="' . htmlspecialchars($winSize[2]) . '"';
+    } else {
+      $string .= ' data-window-tag="' . htmlspecialchars($winSize) . '"';
+    }
+    if ($condtion) {
+     $string .= ' disabled="disabled"';
+    }
+    $string  .=  ($options ? (' ' . $options) : '') . '/>';
   return( $string );
 }
 

web/skins/classic/includes/control_functions.php

@@ -287,7 +287,7 @@ function controlPresets( $monitor, $cmds ) {
   }
   if ( canEdit('Monitors') && $monitor->CanSetPresets() ) {
 ?>
-    <input type="button" class="ptzTextBtn" value="<?php echo translate('Set') ?>" onclick="createPopup('?view=controlpreset&amp;mid=<?php echo $monitor->Id() ?>', 'zmPreset', 'preset');"/>
+    <input type="button" class="ptzTextBtn popup-link" value="<?php echo translate('Set') ?>" data-url="?view=controlpreset&amp;mid=<?php echo $monitor->Id() ?>" data-window-name="zmPreset" data-window-tag="preset"/>
 <?php
   }
 ?>

web/skins/classic/includes/functions.php

@@ -404,8 +404,8 @@ function xhtmlFooter() {
     include("skins/$skin/views/state.php");
   }
 ?>
+  <script>$j('.chosen').chosen();</script>
   </body>
-  <script type="text/javascript">$j('.chosen').chosen();</script>
 </html>
 <?php
 } // end xhtmlFooter

web/skins/classic/js/skin.js

@@ -118,6 +118,26 @@ function createPopup( url, name, tag, width, height ) {
   }
 }
 
+$j(document).ready(function() {
+  $j(".popup-link").click(function onClick(evt) {
+    var el = this;
+    var url;
+    if (el.hasAttribute("href")) {
+      // <a>
+      url = el.getAttribute("href");
+    } else {
+      // buttons
+      url = el.getAttribute("data-url");
+    }
+    var name = el.getAttribute("data-window-name");
+    var tag = el.getAttribute("data-window-tag");
+    var width = el.getAttribute("data-window-width");
+    var height = el.getAttribute("data-window-height");
+    createPopup(url, name, tag, width, height);
+    evt.preventDefault();
+  });
+});
+
 function createEventPopup( eventId, eventFilter, width, height ) {
   var url = '?view=event&eid='+eventId;
   if ( eventFilter )

web/skins/classic/views/controlcaps.php

@@ -63,7 +63,7 @@ foreach( $controls as $control )
 {
 ?>
             <tr>
-              <td class="colName"><?php echo makePopupLink( '?view=controlcap&amp;cid='.$control['Id'], 'zmControlCap', 'controlcap', $control['Name'], canView( 'Control' ) ) ?></td>
+              <td class="colName"><?php echo makePopupLink( '?view=controlcap&cid='.$control['Id'], 'zmControlCap', 'controlcap', $control['Name'], canView( 'Control' ) ) ?></td>
               <td class="colType"><?php echo $control['Type'] ?></td>
               <td class="colProtocol"><?php echo $control['Protocol'] ?></td>
               <td class="colCanMove"><?php echo $control['CanMove']?translate('Yes'):translate('No') ?></td>
@@ -80,7 +80,8 @@ foreach( $controls as $control )
           </tbody>
         </table>
         <div id="contentButtons">
-          <input type="button" value="<?php echo translate('AddNewControl') ?>" onclick="createPopup( '?view=controlcap', 'zmControlCap', 'controlcap' );"<?php if ( !canEdit( 'Control' ) ) {?> disabled="disabled"<?php } ?>/><input type="submit" name="deleteBtn" value="<?php echo translate('Delete') ?>" disabled="disabled"/>
+          <?php echo makePopupButton('?view=controlcap', 'zmControlCap', 'controlcap', translate('AddNewControl'), canEdit( 'Control' )); ?>
+          <input type="submit" name="deleteBtn" value="<?php echo translate('Delete') ?>" disabled="disabled"/>
         </div>
       </form>
     </div>

web/skins/classic/views/devices.php

@@ -75,7 +75,7 @@ foreach( $devices as $device )
           </tbody>
         </table>
         <div id="contentButtons">
-          <input type="button" value="<?php echo translate('New') ?>" onclick="createPopup( '?view=device&amp;did=0', 'zmDevice', 'device' )"<?php echo canEdit('Devices')?'':' disabled="disabled"' ?>/>
+          <?php echo makePopupButton('?view=device&did=0', 'zmDevice', 'device', translate('New'), canEdit( 'Devices' )); ?>
           <input type="button" name="deleteBtn" value="<?php echo translate('Delete') ?>" onclick="deleteDevice( this )" disabled="disabled"/>
           <input type="button" value="<?php echo translate('Cancel') ?>" onclick="closeWindow();"/>
         </div>

web/skins/classic/views/logout.php

@@ -36,9 +36,7 @@ xhtmlHeaders(__FILE__, translate('Logout') );
           <input type="submit" value="<?php echo translate('Logout') ?>"/>
 <?php
 if ( ZM_USER_SELF_EDIT ) {
-?>
+  echo makePopupButton('?view=user&uid=' . $user['Id'], 'zmUser', 'user', translate('Config'));
-          <input type="button" value="<?php echo translate('Config') ?>" onclick="createPopup( '?view=user&amp;uid=<?php echo $user['Id'] ?>', 'zmUser', 'user' );"/>
-<?php
 }
 ?>
           <input type="button" value="<?php echo translate('Cancel') ?>" onclick="closeWindow();"/>

web/skins/classic/views/monitor.php

@@ -468,15 +468,13 @@ if ( canEdit( 'Monitors' ) ) {
   }
 ?>
     <div id="headerButtons">
-      <a href="#" onclick="createPopup( '?view=monitorprobe&amp;mid=<?php echo $monitor->Id()?>', 'zmMonitorProbe<?php echo $monitor->Id()?>', 'monitorprobe' ); return( false );"><?php echo translate('Probe') ?></a>
+      <?php echo makePopupLink('?view=monitorprobe&mid=' . $monitor->Id(), 'zmMonitorProbe' . $monitor->Id(), 'monitorprobe', translate('Probe')); ?>
 <?php
    if ( ZM_HAS_ONVIF ) {
-?>
+       echo makePopupLink('?view=onvifprobe&mid=' . $monitor->Id(), 'zmOnvifProbe' . $monitor->Id(), 'onvifprobe', translate('OnvifProbe'));
-      <a href="#" onclick="createPopup( '?view=onvifprobe&amp;mid=<?php echo $monitor->Id()?>', 'zmOnvifProbe<?php echo $monitor->Id()?>', 'onvifprobe' ); return( false );"><?php echo  translate('OnvifProbe') ?></a>
-<?php
   }
 ?>
-      <a href="#" onclick="createPopup( '?view=monitorpreset&amp;mid=<?php echo $monitor->Id()?>', 'zmMonitorPreset<?php echo $monitor->Id()?>', 'monitorpreset' ); return( false );"><?php echo translate('Presets') ?></a>
+      <?php echo makePopupLink('?view=monitorpreset&mid=' . $monitor->Id(), 'zmMonitorPreset' . $monitor->Id(), 'monitorpreset', translate('Presets')); ?>
     </div>
 <?php
 } // end if canEdit('Monitors')
@@ -975,7 +973,7 @@ if ( $monitor->Type() == 'Local' ) {
     {
 ?>
             <tr><td><?php echo translate('Controllable') ?></td><td><input type="checkbox" name="newMonitor[Controllable]" value="1"<?php if ( $monitor->Controllable() ) { ?> checked="checked"<?php } ?>/></td></tr>
-            <tr><td><?php echo translate('ControlType') ?></td><td><?php echo buildSelect( "newMonitor[ControlId]", $controlTypes, 'loadLocations( this )' ); ?><?php if ( canEdit( 'Control' ) ) { ?>&nbsp;<a href="#" onclick="createPopup( '?view=controlcaps', 'zmControlCaps', 'controlcaps' );"><?php echo translate('Edit') ?></a><?php } ?></td></tr>
+            <tr><td><?php echo translate('ControlType') ?></td><td><?php echo buildSelect( "newMonitor[ControlId]", $controlTypes, 'loadLocations( this )' ); ?><?php if ( canEdit( 'Control' ) ) { ?>&nbsp;<?php echo makePopupLink('?view=controlcaps', 'zmControlCaps', 'controlcaps', translate('Edit')); ?></a><?php } ?></td></tr>
             <tr><td><?php echo translate('ControlDevice') ?></td><td><input type="text" name="newMonitor[ControlDevice]" value="<?php echo validHtmlStr($monitor->ControlDevice()) ?>" size="32"/></td></tr>
             <tr><td><?php echo translate('ControlAddress') ?></td><td><input type="text" name="newMonitor[ControlAddress]" value="<?php echo validHtmlStr($monitor->ControlAddress()) ?>" size="32"/></td></tr>
             <tr><td><?php echo translate('AutoStopTimeout') ?></td><td><input type="text" name="newMonitor[AutoStopTimeout]" value="<?php echo validHtmlStr($monitor->AutoStopTimeout()) ?>" size="4"/></td></tr>

web/skins/classic/views/montage.php

@@ -140,9 +140,7 @@ xhtmlHeaders(__FILE__, translate('Montage'));
         <div id="headerButtons">
 <?php
 if ( $showControl ) {
-?>
+  echo makePopupLink('?view=control', 'zmControl', 'control', translate('Control'));
-        <a href="#" onclick="createPopup('?view=control', 'zmControl', 'control')"><?php echo translate('Control') ?></a>
-<?php
 }
 if ( $showZones ) {
 ?>
@@ -201,9 +199,13 @@ foreach ( $monitors as $monitor ) {
           <div id="monitor<?php echo $monitor->Id() ?>" class="monitor idle">
             <div
               id="imageFeed<?php echo $monitor->Id() ?>"
-              class="imageFeed"
+              class="imageFeed popup-link"
-              onclick="createPopup('?view=watch&amp;mid=<?php echo $monitor->Id() ?>', 'zmWatch<?php echo $monitor->Id() ?>', 'watch', <?php echo reScale( $monitor->Width(), $monitor->PopupScale() ); ?>, <?php echo reScale( $monitor->Height(), $monitor->PopupScale() ); ?> );">
+              data-url="?view=watch&amp;mid=<?php echo $monitor->Id() ?>"
-            <?php 
+              data-name="zmWatch<?php echo $monitor->Id() ?>"
+              data-tag="watch"
+              data-width="<?php echo reScale( $monitor->Width(), $monitor->PopupScale() ); ?>"
+              data-height="<?php echo reScale( $monitor->Height(), $monitor->PopupScale() ); ?>">
+            <?php
   $monitor_options = $options;
   if ( $Positions ) {
     $monitor_options['width'] = '100%';

web/skins/classic/views/options.php

@@ -193,7 +193,7 @@ foreach ( array_map('basename', glob('skins/'.$current_skin.'/css/*',GLOB_ONLYDI
           </tbody>
         </table>
         <div id="contentButtons">
-          <button type="button" value="Add New User" onclick="createPopup('?view=user&amp;uid=0', 'zmUser', 'user');"<?php if ( !canEdit( 'System' ) ) { ?> disabled="disabled"<?php } ?>><?php echo translate('AddNewUser') ?></button>
+        <?php echo makePopupButton('?view=user&uid=0', 'zmUser', 'user', translate("AddNewUser"), canEdit('System')); ?>
           <button type="submit" class="btn-danger" name="deleteBtn" value="Delete" disabled="disabled"><?php echo translate('Delete') ?></button>
         </div>
       </form>
@@ -258,7 +258,7 @@ foreach ( array_map('basename', glob('skins/'.$current_skin.'/css/*',GLOB_ONLYDI
           </tbody>
         </table>
         <div id="contentButtons">
-          <button type="button" value="Add New Server" onclick="createPopup('?view=server&amp;id=0','zmServer','server');"<?php if ( !canEdit( 'System' ) ) { ?> disabled="disabled"<?php } ?>><?php echo translate('AddNewServer') ?></button>
+        <?php echo makePopupButton('?view=server&id=0', 'zmServer', 'server', translate('AddNewServer'), canEdit('System')); ?>
           <button type="submit" class="btn-danger" name="deleteBtn" value="Delete" disabled="disabled"><?php echo translate('Delete') ?></button>
         </div>
       </form>
@@ -299,7 +299,7 @@ foreach ( array_map('basename', glob('skins/'.$current_skin.'/css/*',GLOB_ONLYDI
           </tbody>
         </table>
         <div id="contentButtons">
-          <button type="button" value="Add New Storage" onclick="createPopup('?view=storage&amp;id=0','zmStorage','storage');"<?php if ( !canEdit( 'System' ) ) { ?> disabled="disabled"<?php } ?>><?php echo translate('AddNewStorage') ?></button>
+          <?php echo makePopupButton('?view=storage&id=0', 'zmStorage', 'storage', translate('AddNewStorage'), canEdit('System')); ?>
           <button type="submit" class="btn-danger" name="deleteBtn" value="Delete" disabled="disabled"><?php echo translate('Delete') ?></button>
         </div>
       </form>

web/skins/classic/views/zones.php

@@ -57,7 +57,7 @@ xhtmlHeaders(__FILE__, translate('Zones') );
         <input type="hidden" name="action" value="delete"/>
         <input type="hidden" name="mid" value="<?php echo $mid ?>"/>
         <div id="contentButtons">
-          <input type="button" value="<?php echo translate('AddNewZone') ?>" onclick="createPopup( '?view=zone&amp;mid=<?php echo $mid ?>&amp;zid=0', 'zmZone', 'zone', <?php echo $monitor->Width() ?>, <?php echo $monitor->Height() ?> );"<?php if ( !canEdit( 'Monitors' ) ) { ?> disabled="disabled"<?php } ?>/>
+          <?php echo makePopupButton('?view=zone&mid=' . $mid . '&zid=0', 'zmZone', array('zone', $monitor->Width(), $monitor->Height()), translate('AddNewZone'), canEdit('Monitors')); ?>
           <input type="submit" name="deleteBtn" value="<?php echo translate('Delete') ?>" disabled="disabled"/>
         </div>
         <table id="contentTable" class="major" cellspacing="0">
@@ -74,7 +74,7 @@ xhtmlHeaders(__FILE__, translate('Zones') );
 foreach( $zones as $zone ) {
 ?>
             <tr>
-              <td class="colName"><a href="#" onclick="streamCmdQuit( true ); createPopup( '?view=zone&amp;mid=<?php echo $mid ?>&amp;zid=<?php echo $zone['Id'] ?>', 'zmZone', 'zone', <?php echo $monitor->Width() ?>, <?php echo $monitor->Height() ?> ); return( false );"><?php echo $zone['Name'] ?></a></td>
+              <td class="colName"><?php echo makePopupLink('?view=zone&mid=' . $mid . '&zid=' . $zone['Id'], 'zmZone', array('zone', $monitor->Width(), $monitor->Height()), $zone['Name'], true, 'onclick="streamCmdQuit( true ); return( false );"'); ?></td>
               <td class="colType"><?php echo $zone['Type'] ?></td>
               <td class="colUnits"><?php echo $zone['Area'] ?>&nbsp;/&nbsp;<?php echo sprintf( "%.2f", ($zone['Area']*100)/($monitor->Width()*$monitor->Height()) ) ?></td>
               <td class="colMark"><input type="checkbox" name="markZids[]" value="<?php echo $zone['Id'] ?>" onclick="configureDeleteButton( this );"<?php if ( !canEdit( 'Monitors' ) ) { ?> disabled="disabled"<?php } ?>/></td>
@@ -90,7 +90,12 @@ foreach( $zones as $zone ) {
 <?php
       foreach( array_reverse($zones) as $zone ) {
 ?>
-          <polygon points="<?php echo $zone['AreaCoords'] ?>" class="<?php echo $zone['Type']?>" onclick="streamCmdQuit( true ); createPopup( '?view=zone&amp;mid=<?php echo $mid ?>&amp;zid=<?php echo $zone['Id'] ?>', 'zmZone', 'zone', <?php echo $monitor->Width ?>, <?php echo $monitor->Height ?> ); return( false );"/>
+          <polygon points="<?php echo $zone['AreaCoords'] ?>" class="popup-link <?php echo $zone['Type']?>" onclick="streamCmdQuit( true ); return( false );"
+                   data-url="?view=zone&amp;mid=<?php echo $mid ?>&amp;zid=<?php echo $zone['Id'] ?>"
+                   data-window-name="zmZone"
+                   data-window-tag="zone"
+                   data-window-width="<?php echo $monitor->Width ?>"
+                   data-window-height="<?php echo $monitor->Height ?>"/>
 <?php
       } // end foreach zone
 ?>