LGTM.com-inspired changes:

- Fix: Ensure all apostrophes are escaped for `toXml` utility
- Fix: Avoid error if `URL` is not defined
- Fix (jPicker): Precision argument had not been passed in previously
- Fix (Star extension): Minor: Avoid erring if `inradius` is `NaN`
- Refactoring: Avoid passing unused arguments, setting unused variables,
  and making unnecessary checks; avoid useless call to `createSVGMatrix`
- Linting (LGTM): Add `lgtm.yml` file (still some remaining items flagged
  but hoping for in-code flagging)
- Docs: Contributing file

CHANGES.md

@@ -1,22 +1,32 @@
 # ?
 
-- Known regression for 3.\*: Image libraries [broken](https://github.com/SVG-Edit/svgedit/issues/274)
+- Known regression for 3.\*: Image libraries
+  [broken](https://github.com/SVG-Edit/svgedit/issues/274)
 - Breaking change (minor): Change export to check `exportWindowName`
   for filename and change default from `download` to `svg.pdf` to
   distinguish from other downloads
 - Fix: Given lack of support now for dataURI export in Chrome, provide
   PDF as export (#273 @cuixiping); fixes #124 and #254
+- Fix: Ensure all apostrophes are escaped for `toXml` utility
+- Fix: Avoid error if `URL` is not defined
+- Fix (jPicker): Precision argument had not been passed in previously
 - Fix (image import): Put src after onload to avoid missing event;
   check other width/height properties in case offset is 0; fixes #278
 - Fix (image export): Export in Chrome; fixes #282
 - Fix (Context menus): Avoid showing double shortcuts (#285); add some
   missing ones
+- Fix (Star extension): Minor: Avoid erring if `inradius` is `NaN`
+- Refactoring: Avoid passing unused arguments, setting unused variables,
+  and making unnecessary checks; avoid useless call to `createSVGMatrix`
+- Linting (LGTM): Add `lgtm.yml` file (still some remaining items flagged
+  but hoping for in-code flagging)
+- Linting (ESLint): Consistent spacing; new "standard"
+- Docs: Contributing file
+- Build: Switch to `terser` plugin with `uglify` plugin not
+  supporting ES6+-capable minifier
 - npm: Update devDeps
 - npm: Point to official sinon-test package now that ES6 Modules
     support landed
-- Build: Switch to `terser` plugin with `uglify` plugin not
-    supporting ES6+-capable minifier
-- Linting (ESLint): Consistent spacing; new "standard"
 
 # 3.0.0-rc.2
 

docs/Contributing.md

@@ -0,0 +1,15 @@
+# Contributing
+
+1. Prefix every change in the commit with one of the following types (and
+  sort into this order):
+  - `Security fix: `
+  - `Breaking change: `
+  - `Fix: `
+  - `Fix (<component>): ` Component may be an extension, locale, etc.
+  - `Enhancement: `
+  - `Refactoring: `
+  - `Linting (<type>):` - Linting by type, e.g., "ESLint"
+  - `Docs: `
+  - `Update: ` - e.g., if updating a bundled library
+  - `Build: `
+  - `npm` - Updates to dependencies, npm version, etc.

editor/canvg/canvg.js

@@ -168,9 +168,6 @@ function build (opts) {
     const AJAX = window.XMLHttpRequest
       ? new XMLHttpRequest()
       : new window.ActiveXObject('Microsoft.XMLHTTP');
-    if (!AJAX) {
-      return null;
-    }
     if (asynch) {
       return new Promise((resolve, reject) => {
         const req = AJAX.open('GET', url, true);

editor/extensions/ext-markers.js

@@ -279,7 +279,7 @@ export default {
       const poslist = {start_marker: 'start', mid_marker: 'mid', end_marker: 'end'};
       const pos = poslist[this.id];
       const markerName = 'marker-' + pos;
-      let el = selElems[0];
+      const el = selElems[0];
       const marker = getLinked(el, markerName);
       if (marker) { $(marker).remove(); }
       el.removeAttribute(markerName);
@@ -294,7 +294,9 @@ export default {
       const id = markerPrefix + pos + '_' + el.id;
       addMarker(id, val);
       svgCanvas.changeSelectedAttribute(markerName, 'url(#' + id + ')');
-      if (el.tagName === 'line' && pos === 'mid') { el = convertline(el); }
+      if (el.tagName === 'line' && pos === 'mid') {
+        convertline(el);
+      }
       svgCanvas.call('changed', selElems);
       setIcon(pos, val);
     }

editor/extensions/ext-star.js

@@ -170,7 +170,7 @@ export default {
 
             polyPoints += x + ',' + y + ' ';
 
-            if (inradius != null) {
+            if (!isNaN(inradius)) {
               angle = (2.0 * Math.PI * (s / point)) + (Math.PI / point);
               if (orient === 'point') {
                 angle -= (Math.PI / 2);

editor/jgraduate/jQuery.jPicker.js

@@ -687,7 +687,7 @@ const jPicker = function ($) {
             case 'r':
               if (hsv) continue;
               rgb = true;
-              newV.r = (value && value.r && value.r | 0) || (value && value | 0) || 0;
+              newV.r = (value.r && value.r | 0) || (value | 0) || 0;
               if (newV.r < 0) newV.r = 0;
               else if (newV.r > 255) newV.r = 255;
               if (r !== newV.r) {
@@ -740,7 +740,7 @@ const jPicker = function ($) {
             case 's':
               if (rgb) continue;
               hsv = true;
-              newV.s = value && value.s != null ? value.s | 0 : value != null ? value | 0 : 100;
+              newV.s = value.s != null ? value.s | 0 : value | 0;
               if (newV.s < 0) newV.s = 0;
               else if (newV.s > 100) newV.s = 100;
               if (s !== newV.s) {
@@ -751,7 +751,7 @@ const jPicker = function ($) {
             case 'v':
               if (rgb) continue;
               hsv = true;
-              newV.v = value && value.v != null ? value.v | 0 : value != null ? value | 0 : 100;
+              newV.v = value.v != null ? value.v | 0 : value | 0;
               if (newV.v < 0) newV.v = 0;
               else if (newV.v > 100) newV.v = 100;
               if (v !== newV.v) {
@@ -1659,7 +1659,7 @@ const jPicker = function ($) {
           button = tbody.find('.Button');
         activePreview = preview.find('.Active:first').css({backgroundColor: (hex && '#' + hex) || 'transparent'});
         currentPreview = preview.find('.Current:first').css({backgroundColor: (hex && '#' + hex) || 'transparent'}).bind('click', currentClicked);
-        setAlpha.call($this, currentPreview, Math.precision(color.current.val('a') * 100) / 255, 4);
+        setAlpha.call($this, currentPreview, Math.precision((color.current.val('a') * 100) / 255, 4));
         okButton = button.find('.Ok:first').bind('click', okClicked);
         cancelButton = button.find('.Cancel:first').bind('click', cancelClicked);
         grid = button.find('.Grid:first');
@@ -1687,7 +1687,7 @@ const jPicker = function ($) {
             if (!win.alphaSupport && ahex) ahex = ahex.substring(0, 6) + 'ff';
             const quickHex = color.quickList[i].val('hex');
             if (!ahex) ahex = '00000000';
-            html += '<span class="QuickColor"' + ((ahex && ' title="#' + ahex + '"') || 'none') + ' style="background-color:' + ((quickHex && '#' + quickHex) || '') + ';' + (quickHex ? '' : 'background-image:url(' + images.clientPath + 'NoColor.png)') + (win.alphaSupport && alpha && alpha < 255 ? ';opacity:' + Math.precision(alpha / 255, 4) + ';filter:Alpha(opacity=' + Math.precision(alpha / 2.55, 4) + ')' : '') + '">&nbsp;</span>';
+            html += '<span class="QuickColor"' + (' title="#' + ahex + '"') + ' style="background-color:' + ((quickHex && '#' + quickHex) || '') + ';' + (quickHex ? '' : 'background-image:url(' + images.clientPath + 'NoColor.png)') + (win.alphaSupport && alpha && alpha < 255 ? ';opacity:' + Math.precision(alpha / 255, 4) + ';filter:Alpha(opacity=' + Math.precision(alpha / 2.55, 4) + ')' : '') + '">&nbsp;</span>';
           }
           setImg.call($this, grid, images.clientPath + 'bar-opacity.png');
           grid.html(html);

editor/path.js

@@ -597,7 +597,7 @@ export const getSegSelector = function (seg, update) {
     // Set start point
     replacePathSeg(2, 0, [pt.x, pt.y], segLine);
 
-    const pts = ptObjToArr(seg.type, seg.item, true);
+    const pts = ptObjToArr(seg.type, seg.item); // , true);
     for (let i = 0; i < pts.length; i += 2) {
       const pt = getGripPt(seg, {x: pts[i], y: pts[i + 1]});
       pts[i] = pt.x;
@@ -738,7 +738,7 @@ export class Segment {
    */
   addGrip () {
     this.ptgrip = getPointGrip(this, true);
-    this.ctrlpts = getControlPoints(this, true);
+    this.ctrlpts = getControlPoints(this); // , true);
     this.segsel = getSegSelector(this, true);
   }
 
@@ -1382,7 +1382,7 @@ export const recalcRotatedPath = function () {
   const oldbox = path.oldbbox; // selectedBBoxes[0],
   oldcx = oldbox.x + oldbox.width / 2;
   oldcy = oldbox.y + oldbox.height / 2;
-  let box = getBBox(currentPath);
+  const box = getBBox(currentPath);
   newcx = box.x + box.width / 2;
   newcy = box.y + box.height / 2;
 
@@ -1414,7 +1414,7 @@ export const recalcRotatedPath = function () {
     replacePathSeg(type, i, points);
   } // loop for each point
 
-  box = getBBox(currentPath);
+  /* box = */ getBBox(currentPath);
   // selectedBBoxes[0].x = box.x; selectedBBoxes[0].y = box.y;
   // selectedBBoxes[0].width = box.width; selectedBBoxes[0].height = box.height;
 
@@ -1810,10 +1810,10 @@ export const pathActions = (function () {
         let keep = null;
         let index;
         // if pts array is empty, create path element with M at current point
-        let drawnPath = editorContext_.getDrawnPath();
+        const drawnPath = editorContext_.getDrawnPath();
         if (!drawnPath) {
           const dAttr = 'M' + x + ',' + y + ' '; // Was this meant to work with the other `dAttr`? (was defined globally so adding `var` to at least avoid a global)
-          drawnPath = editorContext_.setDrawnPath(editorContext_.addSVGElementFromJson({
+          /* drawnPath = */ editorContext_.setDrawnPath(editorContext_.addSVGElementFromJson({
             element: 'path',
             curStyles: true,
             attr: {
@@ -1892,7 +1892,7 @@ export const pathActions = (function () {
 
             // This will signal to commit the path
             // const element = newpath; // Other event handlers define own `element`, so this was probably not meant to interact with them or one which shares state (as there were none); I therefore adding a missing `var` to avoid a global
-            drawnPath = editorContext_.setDrawnPath(null);
+            /* drawnPath = */ editorContext_.setDrawnPath(null);
             editorContext_.setStarted(false);
 
             if (subpath) {

editor/recalculate.js

@@ -262,7 +262,7 @@ export const recalculateDimensions = function (selected) {
       box.y + box.height / 2,
       transformListToTransform(tlist).matrix
     );
-    let m = svgroot.createSVGMatrix();
+    // let m = svgroot.createSVGMatrix();
 
     // temporarily strip off the rotate and save the old center
     const gangle = getRotationAngle(selected);
@@ -407,7 +407,7 @@ export const recalculateDimensions = function (selected) {
       tlist.removeItem(N - 3);
     } else if (N >= 3 && tlist.getItem(N - 1).type === 1) {
       operation = 3; // scale
-      m = transformListToTransform(tlist).matrix;
+      const m = transformListToTransform(tlist).matrix;
       const e2t = svgroot.createSVGTransform();
       e2t.setMatrix(m);
       tlist.clear();
@@ -611,7 +611,7 @@ export const recalculateDimensions = function (selected) {
 
     if (!box && selected.tagName !== 'path') return null;
 
-    let m = svgroot.createSVGMatrix();
+    let m; // = svgroot.createSVGMatrix();
     // temporarily strip off the rotate and save the old center
     const angle = getRotationAngle(selected);
     if (angle) {
@@ -737,7 +737,7 @@ export const recalculateDimensions = function (selected) {
     // if it was a rotation, put the rotate back and return without a command
     // (this function has zero work to do for a rotate())
     } else {
-      operation = 4; // rotation
+      // operation = 4; // rotation
       if (angle) {
         const newRot = svgroot.createSVGTransform();
         newRot.setRotate(angle, newcenter.x, newcenter.y);

editor/select.js

@@ -172,7 +172,7 @@ export class Selector {
 
     // apply the transforms
     const l = bbox.x, t = bbox.y, w = bbox.width, h = bbox.height;
-    bbox = {x: l, y: t, width: w, height: h};
+    // bbox = {x: l, y: t, width: w, height: h}; // Not in use
 
     // we need to handle temporary transforms too
     // if skewed, get its transformed box, then find its axis-aligned bbox

editor/svg-editor.js

@@ -3165,7 +3165,7 @@ editor.init = function () {
           parent = '#main_menu ul';
           break;
         }
-        let flyoutHolder, curH, showBtn, refData, refBtn;
+        let flyoutHolder, showBtn, refData, refBtn;
         const button = $((btn.list || btn.type === 'app_menu') ? '<li/>' : '<div/>')
           .attr('id', id)
           .attr('title', btn.title)
@@ -3210,7 +3210,7 @@ editor.init = function () {
             // TODO: Find way to set the current icon using the iconloader if this is not default
 
             // Include data for extension button as well as ref button
-            curH = holders['#' + flyoutHolder[0].id] = [{
+            /* curH = */ holders['#' + flyoutHolder[0].id] = [{
               sel: '#' + id,
               fn: btn.events.click,
               icon: btn.id,
@@ -3271,7 +3271,7 @@ editor.init = function () {
           // TODO: Find way to set the current icon using the iconloader if this is not default
 
           // Include data for extension button as well as ref button
-          curH = holders['#' + flyoutHolder[0].id] = [{
+          const curH = holders['#' + flyoutHolder[0].id] = [{
             sel: '#' + id,
             fn: btn.events.click,
             icon: btn.id,
@@ -4200,7 +4200,7 @@ editor.init = function () {
             </head>
             <body><h1>${str}</h1></body>
           <html>`;
-          if (typeof URL && URL.createObjectURL) {
+          if (typeof URL !== 'undefined' && URL.createObjectURL) {
             const blob = new Blob([popHTML], {type: 'text/html'});
             popURL = URL.createObjectURL(blob);
           } else {
@@ -4286,9 +4286,9 @@ editor.init = function () {
     workarea.toggleClass('wireframe');
 
     if (supportsNonSS) { return; }
-    let wfRules = $('#wireframe_rules');
+    const wfRules = $('#wireframe_rules');
     if (!wfRules.length) {
-      wfRules = $('<style id="wireframe_rules"></style>').appendTo('head');
+      /* wfRules = */ $('<style id="wireframe_rules"></style>').appendTo('head');
     } else {
       wfRules.empty();
     }
@@ -4956,13 +4956,13 @@ editor.init = function () {
     if (sidedrag === -1) { return; }
     sidedragging = true;
     let deltaX = sidedrag - evt.pageX;
-    let sideWidth = $('#sidepanels').width();
+    const sideWidth = $('#sidepanels').width();
     if (sideWidth + deltaX > SIDEPANEL_MAXWIDTH) {
       deltaX = SIDEPANEL_MAXWIDTH - sideWidth;
-      sideWidth = SIDEPANEL_MAXWIDTH;
+      // sideWidth = SIDEPANEL_MAXWIDTH;
     } else if (sideWidth + deltaX < 2) {
       deltaX = 2 - sideWidth;
-      sideWidth = 2;
+      // sideWidth = 2;
     }
     if (deltaX === 0) { return; }
     sidedrag -= deltaX;

editor/svgcanvas.js

@@ -1126,7 +1126,6 @@ this.addExtension = async function (name, extInitFunc, importLocale) {
   if (typeof extInitFunc !== 'function') {
     throw new TypeError('Function argument expected for `svgcanvas.addExtension`');
   }
-  let extObj = {};
   if (!(name in extensions)) {
     // Provide private vars/funcs here. Is there a better way to do this?
     /**
@@ -1148,9 +1147,7 @@ this.addExtension = async function (name, extInitFunc, importLocale) {
       nonce: getCurrentDrawing().getNonce(),
       selectorManager
     });
-    if (extInitFunc) {
+    const extObj = await extInitFunc(argObj);
-      extObj = await extInitFunc(argObj);
-    }
     if (extObj) {
       extObj.name = name;
     }
@@ -1881,7 +1878,8 @@ const mouseDown = function (evt) {
     start.y = realY;
     started = true;
     dAttr = realX + ',' + realY + ' ';
-    strokeW = parseFloat(curShape.stroke_width) === 0 ? 1 : curShape.stroke_width;
+    // Commented out as doing nothing now:
+    // strokeW = parseFloat(curShape.stroke_width) === 0 ? 1 : curShape.stroke_width;
     addSVGElementFromJson({
       element: 'polyline',
       curStyles: true,
@@ -2104,10 +2102,13 @@ const mouseMove = function (evt) {
         dy = snapToGrid(dy);
       }
 
+      /*
+      // Commenting out as currently has no effect
       if (evt.shiftKey) {
         xya = snapToAngle(startX, startY, x, y);
         ({x, y} = xya);
       }
+      */
 
       if (dx !== 0 || dy !== 0) {
         len = selectedElements.length;
@@ -5871,14 +5872,14 @@ this.setImageURL = function (val) {
   if (!elem) { return; }
 
   const attrs = $(elem).attr(['width', 'height']);
-  let setsize = (!attrs.width || !attrs.height);
+  const setsize = (!attrs.width || !attrs.height);
 
   const curHref = getHref(elem);
 
   // Do nothing if no URL change or size change
-  if (curHref !== val) {
+  if (curHref === val && !setsize) {
-    setsize = true;
+    return;
-  } else if (!setsize) { return; }
+  }
 
   const batchCmd = new BatchCommand('Change Image URL');
 
@@ -5887,24 +5888,20 @@ this.setImageURL = function (val) {
     '#href': curHref
   }));
 
-  if (setsize) {
+  $(new Image()).load(function () {
-    $(new Image()).load(function () {
+    const changes = $(elem).attr(['width', 'height']);
-      const changes = $(elem).attr(['width', 'height']);
 
-      $(elem).attr({
+    $(elem).attr({
-        width: this.width,
+      width: this.width,
-        height: this.height
+      height: this.height
-      });
+    });
 
-      selectorManager.requestSelector(elem).resize();
+    selectorManager.requestSelector(elem).resize();
 
-      batchCmd.addSubCommand(new ChangeElementCommand(elem, changes));
+    batchCmd.addSubCommand(new ChangeElementCommand(elem, changes));
-      addCommandToHistory(batchCmd);
-      call('changed', [elem]);
-    }).attr('src', val);
-  } else {
     addCommandToHistory(batchCmd);
-  }
+    call('changed', [elem]);
+  }).attr('src', val);
 };
 
 /**

editor/svgtransformlist.js

@@ -125,7 +125,7 @@ export class SVGTransformList {
     // TODO: how do we capture the undo-ability in the changed transform list?
     this._update = function () {
       let tstr = '';
-      /* const concatMatrix = */ svgroot.createSVGMatrix();
+      // /* const concatMatrix = */ svgroot.createSVGMatrix();
       for (let i = 0; i < this.numberOfItems; ++i) {
         const xform = this._list.getItem(i);
         tstr += transformToString(xform) + ' ';

editor/utilities.js

@@ -112,7 +112,7 @@ export const init = function (editorContext) {
 export const toXml = function (str) {
   // ' is ok in XML, but not HTML
   // > does not normally need escaping, though it can if within a CDATA expression (and preceded by "]]")
-  return str.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/, '&#x27;');
+  return str.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#x27;'); // Note: `&apos;` is XML only
 };
 
 /**
@@ -619,7 +619,7 @@ export const getBBox = function (elem) {
   default:
 
     if (elname === 'use') {
-      ret = groupBBFix(selected, true);
+      ret = groupBBFix(selected); // , true);
     }
     if (elname === 'use' || (elname === 'foreignObject' && isWebkit())) {
       if (!ret) { ret = selected.getBBox(); }

lgtm.yml

@@ -0,0 +1,7 @@
+extraction:
+  javascript:
+    index:
+      filters:
+        - exclude: "editor/xdomain-svgedit-config-iife.js"
+        - exclude: "svgedit-config-iife.js"
+        - exclude: "dist"

screencasts/svgopen2010/script.js

@@ -272,7 +272,7 @@ SlideShow.prototype = {
 };
 
 // Initialize
-const slideshow = new SlideShow(query('.slide')); // eslint-disable-line no-unused-vars
+/* const slideshow = */ new SlideShow(query('.slide')); // eslint-disable-line no-new
 
 document.querySelector('#toggle-counter').addEventListener('click', toggleCounter, false);
 document.querySelector('#toggle-size').addEventListener('click', toggleSize, false);