If you had reached big black overlay with "Install" button then e.g.o.'s role is over already and it's GNOME Shell part is in play now.

Thanks Yuri, updating Product to gnome-shell.

...I find it interesting that the issue is deterministic based on the "type" of extension as displayed on e.g.o, though, since these failures are only occurring for extensions that are grayed out in the listings.

Do you have any thoughts on what's different about those extensions vs. the installable ones, from the perspective of the extensions site, that it would be able to "predict" their failure to install? (Especially given that other extensions, like the Astro Weather mentioned, which also aren't shown in the "Compatible with: Current Version" list for Gnome Shell 3.22, CAN still be successfully installed from e.g.o.)

> I find it interesting that the issue is deterministic based on the "type" of extension

What are you mean by "type"?


For what I tried:

1. Set "disable-extension-version-validation" to true
2. Try to install some unsupported (by version) extension

Expected result: extension installed

Actual result:
Extension is not installed, despite of "Install" overlay was displayed and I clicked "install"

I'm on GNOME 3.20

I can say that there was no exceptions on e.g.o. in same time I was trying to install such unsupported extension.
I will try to debug this a bit more.

*** Bug 725673 has been marked as a duplicate of this bug. ***

OK, so what is going on here.

1. GNOME Shell integration calling "InstallRemoteExtension()" method on "org.gnome.Shell.Extensions" DBus interface with uuid of extension.
2. GNOME Shell query extension info from e.g.o. [1].
From that point GNOME Shell know about extension incompatibility, because it receiving array with all Shell versions that extension supports at e.g.o.
Here is return array example [2]
3. GNOME Shell show popup with install question to user.
4. If user accept installation, GNOME Shell sends download query to e.g.o. [3], but GNOME Shell sends current Shell version again in "shell_version" GET parameter.
5. e.g.o. returns HTTP/404, because there is no such version of extension that supports such GNOME Shell version.

I believe that on step 3 GNOME Shell should inform user:
1. If "disable-extension-version-validation" is disabled - about extension incompatibility.
2. If "disable-extension-version-validation" is enabled - about possible extension incompatibility, but with continue possible.

On step 4 it should not send "shell_version" that is equal to current version. It should send "shell_version" of latest supported by extension Shell version that is fetched on step 2.

[1] https://git.gnome.org/browse/gnome-shell/tree/js/ui/extensionDownloader.js#n28
[2] https://extensions.gnome.org/extension-info/?uuid=a11ykeys@vaina.lt&shell_version=3.20
[3] https://git.gnome.org/browse/gnome-shell/tree/js/ui/extensionDownloader.js#n225

Frank, can you give link to some incompatible (by Shell version) extension that is installs?
As I think no outdated extensions should be installed by Shell.

*** Bug 774682 has been marked as a duplicate of this bug. ***

> As I think no outdated extensions should be installed by Shell.

The Fedora people disagree: https://fedoramagazine.org/fedora-25-released/

> GNOME Shell extensions are also no longer checked for compatibility with the current version of the Shell. This was originally required because the GNOME interfaces were changing rapidly during the early days of GNOME 3. Now these interfaces have stabilized, and extensions can generally be expected to work with new releases.



> Frank, can you give link to some incompatible (by Shell version) extension that is installs?

I can't give you that, but I can give you two extensions where the only change that had to be made is to bump the compatible version:

https://github.com/UshakovVasilii/gnome-shell-extension-freon/
https://github.com/kazysmaster/gnome-shell-extension-lockkeys/

I think that if that's the extent of what extension maintainers have to do most of the time, you are unnecessarily excluding a large number of extensions without an active maintainer that might work just fine.

>> As I think no outdated extensions should be installed by Shell.
> The Fedora people disagree: https://fedoramagazine.org/fedora-25-released/

You are misunderstand me.
Read it as "As I think no outdated extensions should be installed by Shell *for now*".

I'm sure that GNOME Shell should be able to install outdated extensions, by as I see it currently *can not* install such extensions.
And I'm explained why it can not and how I see it should be done.

(In reply to Yuri Konotopov from comment #9)
> You are misunderstand me.

It seems I did. Sorry about that.

*** Bug 776942 has been marked as a duplicate of this bug. ***

Created attachment 343920 [details] [review]
Proposed patch

Here is proposed patch against GNOME Shell 3.20 (I will rebase it on master if it will be accepted).

With this patch:
1. If disable-extension-version-validation is false and extensions do not support current Shell version then DBus error "org.gnome.Shell.ShellVersionError" is returned.
2. If disable-extension-version-validation is true then dialog will be shown to user with message "Extension “%s” do not support current GNOME Shell. Download and install it from extensions.gnome.org?".
3. If user accept download then:
3.1. If minimal supported by extension Shell version is greater than current Shell version then extension with minimal supported Shell version will be installed.
3.2. Otherwise extension with maximum supported Shell version will be installed.

*** Bug 674725 has been marked as a duplicate of this bug. ***

(In reply to Yuri Konotopov from comment #12)
> 2. If disable-extension-version-validation is true then dialog will be shown
> to user with message "Extension “%s” do not support current GNOME Shell.
> Download and install it from extensions.gnome.org?".

I think it should be

> Extension “%s” does not list your version of GNOME Shell as supported.
> Download and install it from extensions.gnome.org anyway?

Fixes:

 * do not => does not
 * Clarify that it may "support" this version in terms of working, but just doesn't list it
 * "current version" was ambiguous whether it's the installed one or the current latest release.
 * Adding "anyway" is nice to somewhat indicate that the user is slightly gambling ;-)

Created attachment 343994 [details] [review]
Proposed patch

Daniel, thanks for review.

Attached updated patch based on Daniel's suggestion. Also added some missing whitespaces

Created attachment 343999 [details] [review]
Proposed patch v3

New patch with proper handling (e.g.o. like) of stable Shell versions.

*** Bug 759880 has been marked as a duplicate of this bug. ***

Any chance to get this in 3.24?

I applied this patch locally to my gnome-shell 3.23.91 and https://extensions.gnome.org/ still shows virtually nothing for me (since it sees there are no extensions compatible with current version).

See https://bugzilla.gnome.org/show_bug.cgi?id=779591 which I think is the same basic issue. gnome-software sees there are no available extensions so it doesn't show the Add-Ons button on its home page.

(In reply to Jeremy Bicha from comment #19)
> https://extensions.gnome.org/ still shows virtually nothing for me (since it
> sees there are no extensions compatible with current version).
> 

I think your issue is another one and may be related to e.g.o. website.

I tried this patch with gnome-shell 3.24.0 on Ubuntu GNOME 17.04. I got an error when I tried to install Dash to Dock from extensions.gnome.org but the Weather extension installed successfully.

https://extensions.gnome.org/extension/613/weather/

> I got an error

So extension was marked with "Error"? Any usefull info in the logs?
This may be incompatibility with GNOME Shell 3.24

Yuri, the website said just "Error" (and Tweak Tool says 'Error loading extension').

More specifically, extensions.gnome.org installed dash-to-dock version 22 which is compatible with gnome-shell 3.8. The latest version is 57 (still in the review queue since it's very new, but version 57 specifically claims 3.24 compatibility) What I'd expect is for version 56 compatible with GNOME 3.22 to be installed when I'm running a newer GNOME version (3.24).

(In reply to Jeremy Bicha from comment #23)
> Yuri, the website said just "Error" (and Tweak Tool says 'Error loading
> extension').
> 

Actual error should be somewhere in logs.

(In reply to Yuri Konotopov from comment #24)
> Actual error should be somewhere in logs.

Yes, but the error isn't important here (the error was that the extension was trying to change something in gnome-shell that was removed a long time ago). The important part is that extensions.gnome.org was installing a very old version of the extension.

Dash to Dock works today because version 57 has been approved, but I think you can find another example if you look around a bit.

Created attachment 348903 [details] [review]
Proposed patch v4

Jeremy, thanks for testing.

Extension version parts were compared as strings so wrong Shell version was selected.
Here is updated patch with fixed versionCompare function.

Review of attachment 343999 [details] [review]:

In addition to the comments below, please provide a proper commit message - subject-only messages are OK for trivial changes like typos, but not for complex changes that involved changing existing API semantics.

::: js/ui/extensionDownloader.js
@@ +47,3 @@
         }
 
+        // Return error if version validation is enabled and extensions do not support current Shell version

This is arguably correct for gnome-software's extension support, but it's wrong for the website:
There's a separate disable-version-check "setting" there (the "current version" / "all versions" combobox), ignoring that because of a hidden dconf setting is confusing.

I think a better solution is to show the dialog with a warning, and toggle the setting if the user proceeds.

@@ +201,3 @@
         this._invocation = invocation;
 
+        this.setShellVersion();

I don't like this, see comment below

@@ +239,2 @@
     _onInstallButtonPressed: function(button, event) {
+        let params = { shell_version: String(this._shell_version) };

I'm not sure I like the semantic change of the parameter from "the shell version" to "the extension version that should be installed", but meh.

I guess in the end it doesn't matter much whether finding the best version is done locally or remotely provided everyone uses the same D-Bus API (which is the case for both plugin and Software). However if we change the semantics like this, we should change them consistently, not have different methods with a 'shell_version' parameter that means different things (that implies removing the parameter where the new meaning doesn't make sense, like extension-info/). Or maybe we should pass the extension version instead, it's weird that we look at the extension-supplied versions/supported-shell-versions information, pick the "best supported shell version" and tell the website to find the matching extension version ...

@@ +279,3 @@
+
+    // Set this._shell_version to Shell version that should be used for download extension.
+    setShellVersion: function() {

I don't like the naming here:
 - it sounds like a setter, but actually takes no arguments and
   updates some private property instead
 - why is this public?
 - _shellVersion, not _shell_version, though
 - "shell version" is ambiguous, as "shell" usually implies
   "gnome-shell", and this isn't the gnome-shell version
   (which is Config.PACKAGE_VERSION, full stop)

So I think something like this reads way easier:

  let supportedShellVersions = Object.keys(this._info.shell_version_map);
  this._mostCompatibleVersion = this._findMostCompatibleVersion(supportedShellVersions);

@@ +283,3 @@
+        // For stable version: "major.minor" and "major.minor.point";
+        // For unstable version only "major.minor.point".
+        function getShellVersions() {

Misnomer - there's no such thing as multiple shell versions.

I can't think of a non-weird name off-hand, so maybe just inline the code. Or better: Add a flag to versionCompare() that determines whether the point component should be used for stable versions (and then use the function in ExtensionUtils.versionCheck() to not implement the same logic in different places). Although then the versionCompare() suggestions below no longer apply :-(

@@ +305,3 @@
+        // Return 0 if version a is equal to version b
+        // Return 1 if version b is greater than version a
+        function versionCompare(a, b) {

This should be a regular helper function, not nested inside another function. It also doesn't work as advertised at the moment, as it uses string comparisons rather than numeric comparisons (that is, "3.8" is bigger than "3.10").

@@ +308,3 @@
+            if (a == b) {
+                return 0;
+            }

Style nit: no braces

@@ +316,3 @@
+                if (a.length < i + 1) {
+                    return -1;
+                }

Dto.

In fact, I suggest something much more concise for the loop:

for (let i = 0; i < Math.max(a.length, b.length); i++) {
    let diff = parseInt(a[i] || "0") - parseInt(b[i] || "0");
    if (diff != 0)
        return diff; // or 'diff / Math.abs(diff);' if you insist on -1,0,1 rather than <0,0,>0
}

@@ +336,3 @@
+        // Use current Shell version if disable-extension-version-validation
+        // is disabled or info array is broken
+        if (!global.settings.get_boolean(ExtensionSystem.EXTENSION_DISABLE_VERSION_CHECK_KEY) ||

As mentioned above, I don't think using the validation key for installation/downloads is right - when we get to this point, the user has already opted in to see incompatible extensions, so failing due to a hidden setting is confusing.

@@ +346,3 @@
+            if(this._info.shell_version_map.hasOwnProperty(version)) {
+                // extensions.gnome.org will strip version if necessary
+                this._shell_version = Config.PACKAGE_VERSION;

Ugh. So on the one hand, gnome-shell looks at the compatible versions and finds the best match, but on the other hand we discard the extension-provided value in some cases and rely on the website to do "the right thing". Pick one or the other please.

@@ +354,3 @@
+        let supported_shell_versions = Object.keys(this._info.shell_version_map);
+        // Sort versions
+        supported_shell_versions.sort(versionCompare);

Style nit: camelCase

@@ +356,3 @@
+        supported_shell_versions.sort(versionCompare);
+
+        if (versionCompare(supported_shell_versions[0], Config.PACKAGE_VERSION) == 1) {

Style nit: no braces

@@ +357,3 @@
+
+        if (versionCompare(supported_shell_versions[0], Config.PACKAGE_VERSION) == 1) {
+            this._shell_version = supported_shell_versions[0];

Wait - if the smallest supported version is less than the current shell version, then use that? If an extension supports ["3.10", "3.20"], we surely want the latter for the current "3.24.0"?

@@ +359,3 @@
+            this._shell_version = supported_shell_versions[0];
+        } else {
+            this._shell_version = supported_shell_versions[supported_shell_versions.length - 1];

That's equally questionable - if an extension supports ["3.10", "3.20"] and we are at version "3.8.4", the former is likely the better match (of course resolving the version locally means that it won't work at all for older gnome-shell versions unless some distro person backports the change)

(In reply to Florian Müllner from comment #27)
> It also doesn't work as advertised at the moment, as it uses
> string comparisons rather than numeric comparisons (that is, "3.8" is bigger
> than "3.10").

That's fixed in the last version, everything else still applies.

While thinking more on this issue it become less clear to me where this should be fixed - in GNOME Shell or at e.g.o.

If we will lower e.g.o. strictness for shell_version field, I think we should do it for newer versions of Shell only.
Florian, what can you say about extensions API stable status - should we lower strictness for >=3.22 or it may be applied for some older versions?

Do we still need to add some warning to Shell users?

(In reply to Yuri Konotopov from comment #29)
> While thinking more on this issue it become less clear to me where this
> should be fixed - in GNOME Shell or at e.g.o.

Right. I think we actually have two separate issues here:

 1. we show a download/install dialog that doesn't do anything

 2. once 1. is fixed, the user can install extensions that won't work
    because version validation is enabled

The latter is very clearly a gnome-shell issue IMHO - after all, it involves a hidden setting that the website is simply unaware of. A possible fix would be to display a dialog in that case that allows the user to toggle the key if the setting is writable, or shows an error message otherwise. Of course any fix would only be available to users in a gnome-shell version that already has the new default, so only users who explicitly turned the version check back on will be affected. I can see a case for sysadmins to allow extensions, but restrict them to exact matches by locking down the version validation setting to avoid problems, but it's clearly a corner case.


That leaves the first issue - thinking more about this, I strongly tend towards a fix on e.g.o. I can see where the version check is coming from - why waste bandwidth on an extension that will not work anyway - but then this has always been a bit half-arsed: Why allow users to see and "install" extensions that won't work? Of course it got worse now that the extension may actually work perfectly fine, if it wasn't for the version check that prevents the download.


> If we will lower e.g.o. strictness for shell_version field, I think we
> should do it for newer versions of Shell only.

I'm not sure about this. To me it makes more sense to only use the shell_version field to find the "best" extension version to download, not to magically fail the download. After all, the download request doesn't come out of nowhere - the user got to a place where she can see and enable an unsupported extension. Don't get me wrong, I can see how a version check makes sense, in particular for older shell versions - I just don't think that the download request is the best place to put it.

What do you think of something along those lines?

 - if the shell is really old, disable the switch for installing
   an extension if the version check fails (and show some explanation
   like "This extension is incompatible with your current GNOME version"
   or so)

 - for some version range (maybe: between 3.12 when the setting was added
   and 3.22 when the default was swapped), show a warning if the version
   check fails, but allow users to install the extension anyway

 - for recent enough versions, just ignore the version check
   (well, probably good to keep the dimmed style in the listing, dunno)


> Florian, what can you say about extensions API stable status - should we
> lower strictness for >=3.22 or it may be applied for some older versions?

"stable" is relative considered that any JS code in gnome-shell is extensions API, but I'd say that the last version that broke a wider range of extensions was 3.10. We've become quite careful to not needlessly break API that extensions might use, but of course where an extension makes changes to a particular shell element, changes to that element may still have broken it (and may still do so in the future). The risk of breakage will never be 0 - even if we stopped making any changing to gnome-shell itself, two extensions that work perfectly fine on their own can step on each others feet when combined - but it has been fairly low for years.

(GNOME 3.24 broke a few extensions because of the switch to mozjs38

https://github.com/eonpatapon/gnome-shell-extension-caffeine/issues/83

The extension works but its Preferences window won't open.)

(In reply to Jeremy Bicha from comment #31)
> (GNOME 3.24 broke a few extensions because of the switch to mozjs38

Ah yes, less outdated SpiderMonkey versions are less forgiving towards bugs. I wouldn't consider that an API break though - the affected code already was incorrect in the past. But you are right, there'll likely be more fallout from that this cycle (it's not like we didn't find broken code in gnome-shell thanks to those changes) - if you are very worried about that from a distributor's point of view, I'm *pretty* certain that you can get away with withholding the gjs update. That is, the changes we had to make to support the updated gjs shouldn't break older versions.

(In reply to Florian Müllner from comment #32)
> I'm *pretty* certain that you can get
> away with withholding the gjs update. That is, the changes we had to make to
> support the updated gjs shouldn't break older versions.

No, I'm pretty sure that gnome-shell 3.24 will not run easily with mozjs24.

I'm not that concerned about mozjs38. I mentioned in the Ubuntu GNOME 17.04 release notes that some extensions will need to be updated to work with mozjs38.

> it involves a hidden setting that the website is simply unaware of

Website is aware of this setting. It was added to NPAPI plugin without plugin API version change at 3.12 development cycle.
However it was not used by e.g.o. until now - I added support for this setting to website while fixing bug 779925.
For < 3.12 versions I assume that Shell can not install unsupported extensions.

So as first step I will hide install buttons from unsupported extensions if disable-extensions-version-validation is enabled.
And as second step I will relax extensions download with shell_version parameter since any download request will be deliberate now. 

So this is completely website issue now.

Florian, thanks for your help.

Fixed and deployed.

Yay \o/