Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning for some commercial mapservice in China #8701

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

bhousel
Copy link
Member

@bhousel bhousel commented Sep 17, 2021

closes #8700

This PR adds some additional checks to the incompatible_source validator to catch some popular map services in China.

@LaoshuBaby - please take a look at the rulesets and let me know if I caught everything (I'm not a native Chinese speaker, but I tried).

It also rewrites the file to be ES6 and simplifies the code a bunch. The new code also embeds the actual matched string in the error message, and it does a better job picking out sources from a semicolon-delimited list and uses the .hash property to disambiguate the issues raised. So for example if someone did say source=google;baidu then we'll see 2 issues raised, not 1.

Screen Shot 2021-09-17 at 11 42 31 AM

@LaoshuBaby
Copy link

LaoshuBaby commented Sep 17, 2021

regex part

(my regex is not good so maybe this is not the minimized)

I can't edit in files directly as what I do in other repo, so I paste my edit at there

    {
      id: 'amap',
      regex: /(amap|autonavi|mapabc|高德(软件|地图|淘金))/i
    },
    {
      id: 'baidu',
      regex: /(baidu|baidumap|mapbar|百度|百度(地图|全景|街景))/i
    },

Sometimes “B” in baidu and “A” in amap maybe uppercase.

Very thank you for conducting so much research for those commercial mapservice, and this is truly a large changes in javascript code, which I can't made by myself.

@bhousel
Copy link
Member Author

bhousel commented Sep 17, 2021

regex: /(baidu|baidumap|百度|百度地图)/i

I guess the original rule /(baidu|百度)/i, should cover any of these cases, because regex matches substrings. Whether the source contains baidu or baidumap they will both match /(baidu|百度)/i

Sometimes “B” in baidu and “A” in amap maybe uppercase.

That's ok - the /i at the end means they will match any case.

Very thank you for conducting so much research for those commercial mapservice, and this is truly a large changes in javascript code, which I can't made by myself.

You're very welcome! 😄

@LaoshuBaby
Copy link

regex: /(baidu|baidumap|百度|百度地图)/i

I guess the original rule /(baidu|百度)/i, should cover any of these cases, because regex matches substrings. Whether the source contains baidu or baidumap they will both match /(baidu|百度)/i

Sometimes “B” in baidu and “A” in amap maybe uppercase.

That's ok - the /i at the end means they will match any case.

Very thank you for conducting so much research for those commercial mapservice, and this is truly a large changes in javascript code, which I can't made by myself.

You're very welcome! 😄

If so, new regex will be

    {
      id: 'amap',
      regex: /(amap|autonavi|mapabc|高德)/i
    },
    {
      id: 'baidu',
      regex: /(baidu|mapbar|百度)/i
    },

because "高德" and "百度" can have a lot of product but they are all belonging to the parent service

@bhousel
Copy link
Member Author

bhousel commented Sep 17, 2021

If so, new regex will be....

Thanks 👍 just updated it...

@simonpoole
Copy link
Contributor

simonpoole commented Oct 28, 2021

@mbrzakovic
Sorry, but regexps for incompatible imagery sources should be added to the OSM api blacklist, not to ID.

See https://github.com/openstreetmap/openstreetmap-website/blob/377f394a7cf968f1937030687fe71a393dea6244/config/settings.yml#L92

@bhousel
Copy link
Member Author

bhousel commented Oct 28, 2021

@simonpoole these are two separate lists. This PR is just for the validator that warns about source tags. We need them here so we can localize the warning message.

If you want them added to the imagery blacklist, you can ask the team that maintains that openstreetmap-website file.
(I don't even know whether these services run any imagery servers to blacklist)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add copyright warning for some commercial mapservice in China
4 participants