Return the correct value for NPNVsupportsCarbonBool

• Component: Plug-ins
• Status: RESOLVED FIXED

[Anders Carlsson]

Return the correct value for NPNVsupportsCarbonBool

[Anders Carlsson]

Created attachment 105226 [details]
Patch

[Darin Adler]

Can we make a regression test for this?

What’s the real world impact?

[Anders Carlsson]

(In reply to comment #2)
> Can we make a regression test for this?

We could, but it would involve making run-webkit-tests aware of whether Carbon plug-ins are available and look for different results depending on that. I don't think it's worth it.

> 
> What’s the real world impact?

Probably none - Carbon plug-ins just assume that the Carbon event model is supported. I just saw that it was wrong and it bothered me.

[Adam Roben (:aroben)]

Comment on attachment 105226 [details]
Patch

This really should have a test associated with it.

[Anders Carlsson]

Created attachment 105236 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 105236 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105236&action=review

> Tools/DumpRenderTree/TestNetscapePlugIn/Tests/mac/SupportsCarbonEventModel.cpp:56
> +    bool testConvert(double x, double y, NPCoordinateSpace sourceSpace, NPCoordinateSpace destSpace)
> +    {
> +        // First convert from src to dest.
> +        double destX, destY;
> +        if (!NPN_ConvertPoint(x, y, sourceSpace, &destX, &destY, destSpace))
> +            return false;
> +
> +        // Then convert back to src
> +        double srcX, srcY;
> +        if (!NPN_ConvertPoint(destX, destY, destSpace, &srcX, &srcY, sourceSpace))
> +            return false;
> +
> +
> +        // Then compare.
> +        if (srcX != x || srcY != y)
> +            return false;
> +
> +        return true;
> +    }

This function never seems to be called.

> Tools/DumpRenderTree/TestNetscapePlugIn/Tests/mac/SupportsCarbonEventModel.cpp:58
> +    bool runTest()

This can be private.

[Anders Carlsson]

Committed r93814: <http://trac.webkit.org/changeset/93814>