1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Fixed BB Code Media Sites callbacks cannot prevent invalid MEDIA tag from being created

Discussion in 'Resolved Bug Reports' started by JoshyPHP, Jun 19, 2014.

  1. JoshyPHP

    JoshyPHP Well-Known Member

    BB Code Media Sites can optionally use a callback to match URLs. When matching a URL, if the callback returns a falsy value XenForo behaves as if it has returned the original "id" value instead. I think this behaviour is incorrect. Here's why.

    The first issue is that it assumes that every URL contains an "id". It's usually not the case for those URLs that require a callback. If the "id" was available in the URL, it probably wouldn't require a callback in the first place, right? If the expression used to match URLs doesn't have a capture named "id", a PHP notice is generated.

    The other issue, which is actually the most important, is that once a URL matches a site there's no way to prevent an invalid MEDIA tag from being created. If a callback returns false (the boolean value, not just a falsy "" or 0), it should mean that the URL does not represent an embeddable media. I think the correct behaviour in this case would be to act as if the URL did not match at all, because otherwise the user ends up with a MEDIA tag that does not work and no idea why.

    I'm not aware of any BB Code Media Site that relies on the incorrect behaviour so I don't think fixing it would break any add-on, but even if it did it's always possible for a callback to return the original "id" value if that's what appropriate. On the other hand, fixing it enables the creation of BB Code Media Sites for sites that don't differentiate the type of content in their URLs. Case in point: Tumblr. Tumblr's URLs are the same regardless of their content: text, photos, videos, they all look the same. It's impossible to embed Tumblr videos only (a request I received recently) and leave Tumblr text posts and images alone.
     
    Adam Howard and MKSwni like this.
  2. Mike

    Mike XenForo Developer Staff Member

    I have changed this as part of 1.4 only, due to the potential for a BC break (likely an edge case, I admit, but still a BC break).
     
    thedude and JoshyPHP like this.

Share This Page