From be17cd249c64ddb93f4bf0bdb5bb144385929973 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Fri, 21 Sep 2018 10:56:07 +0800 Subject: [PATCH] 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 | 18 ++++++++++--- docs/Contributing.md | 15 +++++++++++ editor/canvg/canvg.js | 3 --- editor/extensions/ext-markers.js | 6 +++-- editor/extensions/ext-star.js | 2 +- editor/jgraduate/jQuery.jPicker.js | 10 +++---- editor/path.js | 14 +++++----- editor/recalculate.js | 8 +++--- editor/select.js | 2 +- editor/svg-editor.js | 18 ++++++------- editor/svgcanvas.js | 43 ++++++++++++++---------------- editor/svgtransformlist.js | 2 +- editor/utilities.js | 4 +-- lgtm.yml | 7 +++++ screencasts/svgopen2010/script.js | 2 +- 15 files changed, 91 insertions(+), 63 deletions(-) create mode 100644 docs/Contributing.md create mode 100644 lgtm.yml diff --git a/CHANGES.md b/CHANGES.md index 6b435f3e..7c945e5a 100644 --- a/CHANGES.md +++ b/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 diff --git a/docs/Contributing.md b/docs/Contributing.md new file mode 100644 index 00000000..ca1b4eb3 --- /dev/null +++ b/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 may be an extension, locale, etc. + - `Enhancement: ` + - `Refactoring: ` + - `Linting ():` - Linting by type, e.g., "ESLint" + - `Docs: ` + - `Update: ` - e.g., if updating a bundled library + - `Build: ` + - `npm` - Updates to dependencies, npm version, etc. diff --git a/editor/canvg/canvg.js b/editor/canvg/canvg.js index fadf9164..95f196bc 100644 --- a/editor/canvg/canvg.js +++ b/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); diff --git a/editor/extensions/ext-markers.js b/editor/extensions/ext-markers.js index ba405a75..cde37c69 100644 --- a/editor/extensions/ext-markers.js +++ b/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); } diff --git a/editor/extensions/ext-star.js b/editor/extensions/ext-star.js index 2de606e4..a54a354a 100644 --- a/editor/extensions/ext-star.js +++ b/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); diff --git a/editor/jgraduate/jQuery.jPicker.js b/editor/jgraduate/jQuery.jPicker.js index 886b4c8a..5a23cc2e 100755 --- a/editor/jgraduate/jQuery.jPicker.js +++ b/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 += ' '; + html += ' '; } setImg.call($this, grid, images.clientPath + 'bar-opacity.png'); grid.html(html); diff --git a/editor/path.js b/editor/path.js index fe040491..212b066b 100644 --- a/editor/path.js +++ b/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) { diff --git a/editor/recalculate.js b/editor/recalculate.js index 0d0f7a29..8ce713e3 100644 --- a/editor/recalculate.js +++ b/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); diff --git a/editor/select.js b/editor/select.js index 258e868e..31995b8e 100644 --- a/editor/select.js +++ b/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 diff --git a/editor/svg-editor.js b/editor/svg-editor.js index 1095829b..765e72be 100644 --- a/editor/svg-editor.js +++ b/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') ? '
  • ' : '
    ') .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 () {

    ${str}

    `; - 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 = $('').appendTo('head'); + /* wfRules = */ $('').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; diff --git a/editor/svgcanvas.js b/editor/svgcanvas.js index 2d5c25dd..04a74f04 100644 --- a/editor/svgcanvas.js +++ b/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) { - extObj = await extInitFunc(argObj); - } + const 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) { - setsize = true; - } else if (!setsize) { return; } + if (curHref === val && !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 () { - const changes = $(elem).attr(['width', 'height']); + $(new Image()).load(function () { + const changes = $(elem).attr(['width', 'height']); - $(elem).attr({ - width: this.width, - height: this.height - }); + $(elem).attr({ + width: this.width, + height: this.height + }); - selectorManager.requestSelector(elem).resize(); + selectorManager.requestSelector(elem).resize(); - batchCmd.addSubCommand(new ChangeElementCommand(elem, changes)); - addCommandToHistory(batchCmd); - call('changed', [elem]); - }).attr('src', val); - } else { + batchCmd.addSubCommand(new ChangeElementCommand(elem, changes)); addCommandToHistory(batchCmd); - } + call('changed', [elem]); + }).attr('src', val); }; /** diff --git a/editor/svgtransformlist.js b/editor/svgtransformlist.js index a5872b0b..3e209e0f 100644 --- a/editor/svgtransformlist.js +++ b/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) + ' '; diff --git a/editor/utilities.js b/editor/utilities.js index bdd97743..a3d82fb9 100644 --- a/editor/utilities.js +++ b/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, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/, '''); + return str.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, '''); // Note: `'` 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(); } diff --git a/lgtm.yml b/lgtm.yml new file mode 100644 index 00000000..c1d4ef7b --- /dev/null +++ b/lgtm.yml @@ -0,0 +1,7 @@ +extraction: + javascript: + index: + filters: + - exclude: "editor/xdomain-svgedit-config-iife.js" + - exclude: "svgedit-config-iife.js" + - exclude: "dist" diff --git a/screencasts/svgopen2010/script.js b/screencasts/svgopen2010/script.js index 5dcb7882..81eaa1a5 100644 --- a/screencasts/svgopen2010/script.js +++ b/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);