Fix communication with DING

[sergio-costas]

The extension state naming has changed from gnome shell 45 to gnome shell 46, so the code to notify margins to DING wasn't being able to detect when an extension was active, and so it didn't prevent to put icons below the dock.

This patch fixes it.

[sergio-costas]

@3v1n0 Can you review this when you have some spare time, please?

[smedir]

a. Why not just active? that would already means enabled.
b. What is happening right now? Gtk4-Ding is not having any issues at this moment on my desktop. Are there any errors being logged by dash-to-dock?

[sergio-costas]

@smedir Active is the new name in Gnome46 for what in Gnome45 was Enabled, so if we want to keep it working in both versions, both cases must be taken into account.

What happens is that if you have the intellihide option enabled, icons can be put under the dock, where they can be inaccessible unless you put a small window covering the dock in the opposite size, in order to hide it. This also happens with dash-to-panel and hide-top-bar.

[smedir]

if (usableArea?.uuid === IDENTIFIER_UUID) {

should fail automatically if the extension is not 'active', ie the enable method is not called, even when it is loaded, ie 'enabled', as the desktopIconItegrations class is supposed to be called and initialized when the extension is enabled, and nulled when disabled. Therefore usableArea should theoretically not exist at all.

So what I was wondering is where and how is this happening, and what, if any, error is being logged.

[sergio-costas]

@smedir The point is that you are supposing that an extensions is behaving correctly, which may not be the case, so I prefer to be cautious and check before if it is really enabled.

[smedir]

I essence, the code could be simplified to -

_sendMarginsToExtension(extension) {
        // check that the extension is an extension that has the logic to accept
        // working margins
        const usableArea = extension?.stateObj?.DesktopIconsUsableArea;

        if (usableArea?.uuid === IDENTIFIER_UUID)
            usableArea.setMarginsForExtension(this._myUUID, this._margins);
    }

This already makes sure the extension is 'enabled' and 'active'.

And I am not sure why the icons are coming under the dock, as this should be working properly. The error must be because of some other problem.

[sergio-costas]

@smedir The point is that up to Gnome45, it was used "ExtensionState.ENABLED" to specify that an extension was enabled; but in Gnome46 the Enum was changed, and that option was renamed to "ExtensionState.ACTIVE", so the original code compared the current state with "Undefined", thus always was FALSE, and that's why DING never received the margins data from Dash-to-Dock.

[sergio-costas]

In this PR I compare with both values to ensure that it works both in Gnome45 and Gnome46.

[smedir]

Looking at the original MR in Gnome shell here

45 has ENABLED vs DISABLED
46 has INACTIVE vs ACTIVE

The correct code would therefore be

if (!(extension?.state === ExtensionUtils.ExtensionState.ENABLED) ||
       (extension?.state === ExtensionUtils.ExtensionState.ACTIVE))

This also needs to be corrected on line 78 in the constructor as well, where we are connecting to the signal for state changes and then sending margins to extension.

Or if we wish to simplify the code and prevent any problems from state 'names' in future, just eliminate this check in line 78 constructor and sendMarginsToExtension, as const usableArea = extension?.stateObj?.DesktopIconsUsableArea; checks for this already.

[sergio-costas]

By DeMorgan, !(X or Y) = !X AND !Y ;-)

[sergio-costas]

But you are right about line 78, I have to fix that too.

[smedir]

In

By DeMorgan, !(X or Y) = !X AND !Y ;-)

😄 I would still suggest writing it !(X or Y) as it is more intuitive when reviewing the code, as that is what we are actually doing.

Or eliminating it altogether as I suggested as the usableArea? should take care of it.

[sergio-costas]

I think this is a simple fix.

[smedir]

Ah! I think that is much, much better, more intuitive to read and understand.

Strange placement for the _check...() method.

Would you also submit MR for dash-to-panel and other extensions using this class?

[sergio-costas]

I sent the path for dash-to-panel and hide-top-bar five minutes ago :-)

[vanvugt]

Ugh sorry for the 3 commit mess @3v1n0 and others. I approved this reviewing the diff alone, and apparently assumed it was a single commit like #2221.