[Webkit-unassigned] [Bug 24988] The Webkit plugin mime type matching code should allow the wildcards

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24988


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29194|review?                     |review-
               Flag|                            |




------- Comment #6 from darin at apple.com  2009-04-02 10:36 PDT -------
(From update of attachment 29194)
> +        if (m_mimes[i]->type == mimeType) {
> +            return true;
> +        } else if (allowWildCard && m_mimes[i]->type == "*") {
>              return true;
> +        }

Single line if statements bodies don't get braces in the WebKit coding style
(documented in the style document).

No else after return in the WebKit coding style.

> +    PluginPackage* pluginPackageSupportingWildcards = NULL;

WebKit coding style uses 0, not NULL.

> +            // We check for exact mime type matches and wildcard (*) matches. 
> +            if ((mimeiter->first == key) || (mimeiter->first == "*"))

We normally don't use the extra parentheses as done here.

> -    if (pluginChoices.isEmpty())
> +    if (pluginChoices.isEmpty()) {
>          return 0;
> +    }

This changes code that's correct according to the WebKit style to a different
style, perhaps one you prefer. But we have a standard you should adhere to.

>      } else if (fileName() == "npmozax.dll")
>          // Bug 15217: Mozilla ActiveX control complains about missing xpcom_core.dll
>          return true;
> +      else if (fileName() == "npnul32.dll")
> +        // We don't want to load the Mozilla NULL plugin, which is used to install
> +        // NPAPI plugins.
> +        return true;

These multiple line bodies need braces to fit the style. Both the npmozax.dll
case that's already.

I don't think the name of the plug-in is "NULL". You can call it the "null
plug-in" instead of the "NULL plugin".

> -    return pluginData && !type.isEmpty() && pluginData->supportsMimeType(type);
> +    // We pass in true to the supportsMimeType function in PluginData to allow
> +    // plugins which support the wildcard mime type * to be instantiated.
> +    return pluginData && !type.isEmpty() && pluginData->supportsMimeType(type, true);

We want to avoid having to write two-line comments just to explain the boolean,
so please take my suggestion about not using a boolean.

Do we want other styles of wildcard to work, such as "text/*", or is it just
"*" that we need?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.