diff --git a/CHANGELOG.md b/CHANGELOG.md index fb12d312887..08335c39fa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,8 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :scissors: Operations #### :camera: Street-Level #### :white_check_mark: Validation +* Show warnings for potentially incompatible sources also in changeset `source` tags ([#11334]) +* Include the number of changeset tags with incompatible sources in `warnings:incompatible_source` changeset tag ([#8400]) #### :bug: Bugfixes #### :earth_asia: Localization #### :hourglass: Performance @@ -49,7 +51,9 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :mortar_board: Walkthrough / Help #### :hammer: Development +[#8400]: https://github.com/openstreetmap/iD/issues/8400 [#11141]: https://github.com/openstreetmap/iD/pull/11141 +[#11334]: https://github.com/openstreetmap/iD/pull/11334 [#11353]: https://github.com/openstreetmap/iD/pull/11353 diff --git a/data/core.yaml b/data/core.yaml index 0063c29e3c2..2e6673e8eca 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -623,8 +623,10 @@ en: about_changeset_comments: About changeset comments about_changeset_comments_link: //wiki.openstreetmap.org/wiki/Good_changeset_comments changeset_comment_length_warning: "Changeset comments can have a maximum of {maxChars} characters." - google_warning: "You mentioned Google in this comment: remember that copying from Google Maps is strictly forbidden." - google_warning_link: https://www.openstreetmap.org/copyright + changeset_incompatible_source: + comment: 'You mentioned "{value}" in this changeset comment.' + source: 'You mentioned "{value}" as a source of this changeset.' + link: https://www.openstreetmap.org/copyright contributors: list: "Edits by {users}" truncated_list: diff --git a/modules/ui/changeset_editor.js b/modules/ui/changeset_editor.js index 670ced36c6e..1ef8259c5f7 100644 --- a/modules/ui/changeset_editor.js +++ b/modules/ui/changeset_editor.js @@ -8,6 +8,7 @@ import { uiCombobox} from './combobox'; import { uiField } from './field'; import { uiFormFields } from './form_fields'; import { utilArrayUniqBy, utilCleanOsmString, utilRebind, utilTriggerEvent, utilUnicodeCharsCount } from '../util'; +import { getIncompatibleSources } from '../validations/incompatible_source'; export function uiChangesetEditor(context) { @@ -97,58 +98,73 @@ export function uiChangesetEditor(context) { } } - // Show warning(s) if comment mentions Google or comment length exceeds 255 chars - const warnings = []; - if (_tags.comment?.match(/google/i)) { - warnings.push({ - id: 'contains "google"', - msg: t.append('commit.google_warning'), - link: t('commit.google_warning_link') + function findIncompatibleSources(str, which) { + const incompatibleSources = getIncompatibleSources(str); + if (!incompatibleSources) return false; + return incompatibleSources.map(rule => { + const value = rule.regex.exec(str)[1]; + return { + id: `incompatible_source.${which}.${rule.id}`, + msg: selection => { + selection.call(t.append(`commit.changeset_incompatible_source.${which}`, { value })); + selection.append('br'); + selection + .append('a') + .attr('href', t('commit.changeset_incompatible_source.link')) + .call(t.append(`issues.incompatible_source.reference.${rule.id}`)); + } + }; }); } + + function renderWarnings(warnings, selection, klass) { + const entries = selection.selectAll(`.${klass}`) + .data(warnings, d => d.id); + + entries.exit() + .transition() + .duration(200) + .style('opacity', 0) + .remove(); + + const enter = entries.enter() + .append('div') + .classed('field-warning', true) + .classed(klass, true) + .style('opacity', 0); + + enter + .call(svgIcon('#iD-icon-alert', 'inline')) + .append('span'); + + enter + .transition() + .duration(200) + .style('opacity', 1); + + entries.merge(enter).selectAll('div > span') + .text('') + .each(function(d) { + d3_select(this).call(d.msg); + }); + } + + // Show warning(s) if comment mentions an invalid source + const commentWarnings = findIncompatibleSources(_tags.comment, 'comment'); + // also show warning when comment length exceeds 255 chars const maxChars = context.maxCharsForTagValue(); const strLen = utilUnicodeCharsCount(utilCleanOsmString(_tags.comment, Number.POSITIVE_INFINITY)); if (strLen > maxChars || !true) { - warnings.push({ + commentWarnings.push({ id: 'message too long', msg: t.append('commit.changeset_comment_length_warning', { maxChars: maxChars }), }); } + renderWarnings(commentWarnings, selection.select('.form-field-comment'), 'comment-warning'); - var commentWarning = selection.select('.form-field-comment').selectAll('.comment-warning') - .data(warnings, d => d.id); - - commentWarning.exit() - .transition() - .duration(200) - .style('opacity', 0) - .remove(); - - var commentEnter = commentWarning.enter() - .insert('div', '.comment-warning') - .attr('class', 'comment-warning field-warning') - .style('opacity', 0); - - commentEnter - .call(svgIcon('#iD-icon-alert', 'inline')) - .append('span'); - - commentEnter - .transition() - .duration(200) - .style('opacity', 1); - - commentWarning.merge(commentEnter).selectAll('div > span') - .text('') - .each(function(d) { - let selection = d3_select(this); - if (d.link) { - selection = selection.append('a') - .attr('target', '_blank') - .attr('href', d.link); - } - selection.call(d.msg); - }); + // Show warning(s) if sources contain an invalid source + const sourceWarnings = findIncompatibleSources(_tags.source, 'source'); + renderWarnings(sourceWarnings, selection.select('.form-field-source'), 'source-warning'); } diff --git a/modules/ui/commit.js b/modules/ui/commit.js index ed1b779ca3c..0aeaf899e02 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -14,6 +14,7 @@ import { uiCommitWarnings } from './commit_warnings'; import { uiSectionRawTagEditor } from './sections/raw_tag_editor'; import { utilArrayGroupBy, utilRebind, utilUniqueDomId } from '../util'; import { utilDetect } from '../util/detect'; +import { getIncompatibleSources } from '../validations/incompatible_source'; var readOnlyTags = [ @@ -55,8 +56,6 @@ export function uiCommit(context) { // Initialize changeset if one does not exist yet. if (!context.changeset) initChangeset(); - loadDerivedChangesetTags(); - selection.call(render); } @@ -185,11 +184,18 @@ export function uiCommit(context) { } // add counts of warnings generated by the user's edits - var warnings = context.validator() + const warnings = context.validator() .getIssuesBySeverity({ what: 'edited', where: 'all', includeIgnored: true, includeDisabledRules: true }) .warning .filter(function(issue) { return issue.type !== 'help_request'; }); // exclude 'fixme' and similar - #8603 + // also include "incompatible_source" warnings caused by changeset tags + [ ...getIncompatibleSources(tags.comment), + ...getIncompatibleSources(tags.source) + ].forEach(() => { + warnings.push({ type: 'incompatible_source' }); + }); + addIssueCounts(warnings, 'warnings'); // add counts of issues resolved by the user's edits @@ -200,6 +206,7 @@ export function uiCommit(context) { } function render(selection) { + loadDerivedChangesetTags(); var osm = context.connection(); if (!osm) return; diff --git a/modules/validations/incompatible_source.js b/modules/validations/incompatible_source.js index 4d5841e9d42..458a036b841 100644 --- a/modules/validations/incompatible_source.js +++ b/modules/validations/incompatible_source.js @@ -2,25 +2,36 @@ import { t } from '../core/localizer'; import { utilDisplayLabel } from '../util/utilDisplayLabel'; import { validationIssue, validationIssueFix } from '../core/validation'; +const incompatibleRules = [ + { + id: 'amap', + regex: /(^amap$|^amap\.com|autonavi|mapabc|高德)/i + }, + { + id: 'baidu', + regex: /(baidu|mapbar|百度)/i + }, + { + id: 'google', + regex: /(google)/i, + exceptRegex: /((books|drive)\.google|google\s?(books|drive|plus))|(esri\/Google_Africa_Buildings)/i + } +]; + +/** + * @param {string} str String (e.g. tag value) to check for incompatible sources + * @returns {{id:string, regex: RegExp, exceptRegex?: RegExp}[]} + */ +export function getIncompatibleSources(str) { + return incompatibleRules + .filter(rule => + rule.regex.test(str) && + !rule.exceptRegex?.test(str) + ); +} export function validationIncompatibleSource() { const type = 'incompatible_source'; - const incompatibleRules = [ - { - id: 'amap', - regex: /(^amap$|^amap\.com|autonavi|mapabc|高德)/i - }, - { - id: 'baidu', - regex: /(baidu|mapbar|百度)/i - }, - { - id: 'google', - regex: /google/i, - exceptRegex: /((books|drive)\.google|google\s?(books|drive|plus))|(esri\/Google_Africa_Buildings)/i - } - ]; - const validation = function checkIncompatibleSource(entity) { const entitySources = entity.tags && entity.tags.source && entity.tags.source.split(';'); @@ -29,16 +40,8 @@ export function validationIncompatibleSource() { const entityID = entity.id; return entitySources - .map(source => { - const matchRule = incompatibleRules.find(rule => { - if (!rule.regex.test(source)) return false; - if (rule.exceptRegex && rule.exceptRegex.test(source)) return false; - return true; - }); - - if (!matchRule) return null; - - return new validationIssue({ + .flatMap(source => getIncompatibleSources(source) + .map(matchRule => new validationIssue({ type: type, severity: 'warning', message: (context) => { @@ -56,10 +59,8 @@ export function validationIncompatibleSource() { new validationIssueFix({ title: t.append('issues.fix.remove_proprietary_data.title') }) ]; } - }); - - }).filter(Boolean); - + })) + ); function getReference(id) { return function showReference(selection) {