MBS-9515: Block anchors in new Wikipedia URLs #562
Conversation
| } else if ((!isPositiveInteger(link.relationship) || (oldLink && link.url !== oldLink.url)) && /^(https?:\/\/)?([^.\/]+\.)?wikipedia\.org\/.*#/.test(link.url)) { | ||
| // Kludge for MBS-9515 to be replaced with the more general MBS-9516 | ||
| error = l('Anchor (#Example_section) Wikipedia links are not allowed, see the {url|guidelines for URL}.', { | ||
| url: { href: '/doc/Style/Relationships/URLs', target: '_blank' } |
There was a problem hiding this comment.
This should probably link to something like https://musicbrainz.org/relationship/29651736-fa6d-48e4-aadc-a557c6add1cb
It's kind of annoying that we don't have a central "Wikipedia page" rel to link to, but only one per entity type :/ But linking to a specific one, like artist, might be better than nothing.
yvanzo
force-pushed
the
mbs-9515-block-wp-anchor
branch
from
b684725
to
4771d96
Compare
| @@ -196,6 +196,12 @@ class ExternalLinksEditor extends React.Component<LinksEditorProps, LinksEditorS | |||
| error = l('This relationship type is deprecated and should not be used.'); | |||
| } else if ((!isPositiveInteger(link.relationship) || (oldLink && link.url !== oldLink.url)) && checker && !checker(link.url)) { | |||
| error = l('This URL is not allowed for the selected link type, or is incorrectly formatted.'); | |||
| } else if ((!isPositiveInteger(link.relationship) || (oldLink && link.url !== oldLink.url)) && /^(https?:\/\/)?([^.\/]+\.)?wikipedia\.org\/.*#/.test(link.url)) { | |||
| // Kludge for MBS-9515 to be replaced with the more general MBS-9516 | |||
| error = l('Anchors Wikipedia links are not allowed, please remove {fragment} if still appropriate, see the {url|guidelines}.', { | |||
There was a problem hiding this comment.
Will some users know what an anchor is? I guess it's fine, but I wonder if something like "Wikipedia links to specific sections of the page are not allowed, ..." would make more sense.
The comma splice at the end sounds slightly unnatural so I'd replace "appropriate, see" with "appropriate. See" too.
There was a problem hiding this comment.
Almost applied in 46feadc
| } else if ((!isPositiveInteger(link.relationship) || (oldLink && link.url !== oldLink.url)) && /^(https?:\/\/)?([^.\/]+\.)?wikipedia\.org\/.*#/.test(link.url)) { | ||
| // Kludge for MBS-9515 to be replaced with the more general MBS-9516 | ||
| error = l('Anchors Wikipedia links are not allowed, please remove {fragment} if still appropriate, see the {url|guidelines}.', { | ||
| fragment: '<i>' + _.escape(link.url.replace(/^(?:https?:\/\/)?(?:[^.\/]+\.)?wikipedia\.org\/[^#]*#(.*)$/, '#$1')) + '</i>', |
There was a problem hiding this comment.
It's much safer to pass __react: true here and pass in a React element directly, like fragment: <i>{link.url.replace(...)}</li>. Then you can revert the dangerouslySetInnerHTML below.
There was a problem hiding this comment.
Note that for the URL syntax you don't need to pass in an <a>, the i18n module will do that for you with __react: true. So the url: {...} part can stay as it is.
yvanzo
force-pushed
the
mbs-9515-block-wp-anchor
branch
from
4771d96
to
46feadc
Compare
|
Selenium tests seem to deadlock build #631. |
MBS-9515: Block adding Wikipedia relationships to anchors