Bug 173436 – Web Inspector: Only enable the "Show Grid" navigation item if the image has transparent pixels

• Component: Web Inspector
• Status: RESOLVED WONTFIX

[Devin Rousso]

This will prevent awkward scenarios where the user clicks on the button, only for it to have no effect since the grid is not visible.

[Devin Rousso]

Created attachment 313015 [details]
Patch

[Devin Rousso]

Created attachment 313016 [details]
Patch

Oops.  Included some extra stuff :P

[Joseph Pecoraro]

Comment on attachment 313016 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:55
> +WebInspector.imageHasTransparency = function(imageElement, skip = 100)

I think this may be unnecessarily complex. I'd think the expensive part would be getImageData, not looping over to detect if the alpha bit is < 255. So the skipping might not be necessary and might just lead to misdiagnosing an image with transparency.

I think we should also be able to write a simple test for this with some tiny images.

> Source/WebInspectorUI/UserInterface/Base/ImageUtilities.js:71
> +    for (let i = 3; i < imageData.data.length; i += 4 * skip) {

If `skip` is 0 then this will infinite loop. Seems like it is easy to mis-use this function.

> Source/WebInspectorUI/UserInterface/Views/ImageResourceContentView.js:70
> +        this._imageElement.addEventListener("load", (event) => {
> +            let imageHasTransparency = WebInspector.imageHasTransparency(this._imageElement);
> +            this._showGridButtonNavigationItem.enabled = imageHasTransparency;
> +            if (imageHasTransparency)
> +                this._toggleImageGrid();
> +
> +            URL.revokeObjectURL(objectURL);
> +        });

Style: Put this after setting the more direct attributes. The code is easier to read that way.

[Joseph Pecoraro]

After discussion we may just not want to take this approach at all and just always leave the button enabled?

[Devin Rousso]

(In reply to Joseph Pecoraro from comment #4)
> After discussion we may just not want to take this approach at all and just
> always leave the button enabled?

Agreed.  In the case that the image has no transparency, the fact that clicking the Show Grid button has no visible effect is a good indicator in itself.