Making WordPress.org

Opened 5 years ago

Closed 5 years ago

#5083 closed defect (bug) (fixed)

Plugin Directory: Improve Trademark checks

Reported by: ipstenu's profile Ipstenu Owned by: coffee2code's profile coffee2code
Milestone: Priority: normal
Component: Plugin Directory Keywords: has-patch
Cc:

Description

A few other trademark owners have been hitting the abused list, so we're adding some new stoppers.

Also splitting the trademark block notice to differentiate between ones like 'Facebook' which prohibit the use at all and others that allow you to end with the term. Hopefully this will reduce the complaint emails about why they can't use instagram at all (though it's unlikely to stop people from trying to be clever with 'in-sta-gram' :sigh:)

Attachments (2)

trademark-changes.diff (6.5 KB) - added by Ipstenu 5 years ago.
5083.2.diff (7.0 KB) - added by Ipstenu 5 years ago.
Updated with Scott's notes

Download all attachments as: .zip

Change History (12)

#1 @Otto42
5 years ago

Why get rid of the trim() in line 411?

#2 @Ipstenu
5 years ago

In order to do a different check on line 120.

Instead of just removing the trailing hyphen, we check if the slug is the same with and without the trim. If it's the same, then we blocked the whole WORD and not just the placement (the difference between facebook which you can't use at all anymore and woocommerce, which you can use at the end). If we blocked the whole word, I want them to get a slightly different message which (maybe) will be less confusing. We've had an uptick in people asking "But I put in FOR Facebook!!!"

This ticket was mentioned in Slack in #meta by ipstenu. View the logs.


5 years ago

#4 @Ipstenu
5 years ago

@Otto42 With the change requested by certain trademark owners, it'd be nice to get this in sooner :)

#5 @coffee2code
5 years ago

  • Keywords has-patch added

Regarding the check performed in has_trademarked_slug(), wouldn't it be better to differentiate the two types of intended checking being implied Terms that end in "-" denote terms that cannot be the start of a plugin name. For instance, we have "bing-". However, the check is being performed via strpos(), so that actually checks anywhere in the string. A hypothetical slug "tabbing-controls" would fail this check.

For trademark terms not ending in "-", the strpos() check is valid, assuming we don't add too generic of a term. (e.g. "woo-" is valid to denote something starting with "woo", but "woo" as a term is bad since it would match too many valid words). Using preg_match() and checking for word boundaries would be more precise, but perhaps overkill, as long as trademarks don't balloon.

Anyhow, proposed change for the relevant section of has_trademarked_slug():

        foreach ( $trademarked_slugs as $trademark ) {
            // Trademarks ending in "-" indicate slug cannot begin with that term.
            if ( '-' === $trademark[-1] ) {
                if ( 0 === strpos( $this->plugin_slug, $trademark ) ) {
                    $has_trademarked_slug = $trademark;
                    break;
                }
            }
            // Otherwise, the term cannot appear anywhere in slug.
            elseif ( false !== strpos( $this->plugin_slug, $trademark ) ) {
                $has_trademarked_slug = $trademark;
                break;
            }
        }

Assuming that change looks good and I haven't misunderstood things, I think the patch + my changes look good.

#6 @Ipstenu
5 years ago

That would solve one part of the problem, but the other is I wanted to have separate messages for people.

Problem 1 - banned words

Facebook won't allow facebook (or FB or Face in relation to Facebook, see also Insta and yes that drives me to drink) anywhere in a permalink or name. Similarly, we're trying to discourage 'Gutenberg' in plugin slugs as they can't be changed and that's the project name, not the editor name. In those cases, people should get hard stopped and told "Hi, you're using a banned word."

Problem 2 - banned 'start' words

Brand owners would like to protect the start word (woocommerce-, bing-, microsoft- etc). Those should be told "Hi, you can't START with that term, please try again."

Problem 3 - portmanteaus

This one is NOT touched by this patch, but the issue there is, for example, "WordBook" -- if that was a Facebook/WordPress integration plugin, Facebook would like to stop you. It's something we just have to be vigilant for since it's hard to divine intent here :/

#7 @coffee2code
5 years ago

I'm on board with that. I was merely amending your patch (or perhaps more accurately proposing a follow-up patch) to make the "Problem 2" check actually only check that the trademark is at the beginning of the slug as opposed to anywhere in the slug (e.g. checking strpos() returns literal 0 instead of simply not being false, as currently being checked).

I didn't intend to suggest that my code snippet above was to preclude your separate messaging for banned words and banned start words. So it sounds like we're in agreement.

#8 @Ipstenu
5 years ago

Ah! I misread where the change was and what it changed. Cool cool. Ammending and will upload in a tick.

@Ipstenu
5 years ago

Updated with Scott's notes

#9 @Ipstenu
5 years ago

Patch refreshed.

#10 @coffee2code
5 years ago

  • Owner set to coffee2code
  • Resolution set to fixed
  • Status changed from new to closed

In 9613:

Plugin Directory, Upload Handler: Improve trademark checks.

  • Differentiate error message based on whether trademark cannot be used at the start of the plugin name or at all
  • Amend the list of trademarks
  • Adjust trademark detection so that trademarks that cannot be used at the start of the plugin name are more strictly checked for only that position

Props Ipstenu, coffee2code.
Fixes #5083.

Note: See TracTickets for help on using tickets.