672587 - Fx 7 compatibility results show warnings related to Firefox 6

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[krupa raj[:krupa]]

Attachment: screenshot

steps to reproduce:
1. Load https://addons.allizom.org/en-US/developers/addon/tfox/validation-result/38008
2. Notice the warnings


expected behavior:
warnings and errors shown are applicable to the app version being tested.

actual behavior:
Fx 7 compatibility results show warnings related to Firefox 6. See screenshot

Comment 1

[Matt Basta [:basta]]

Correct me if I'm wrong, but FX6 compatibility problems are still going to affect add-ons that target FX7.

As far as I know, Zamboni just sets the maxVersion number. Ideally, to target only FX7, the minVersion should also be overridden. The range of supported versions should be the range of compatibility tests that are run.

Comment 2

[Wil Clouser [:clouserw]]

Jorge, what do you want to do here?

Feels like not showing the warnings is more technically correct, but from the author's point of view I think the current behavior is best.

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

If the add-on is already compatible with 6.* (or higher), I think we shouldn't be showing Firefox 6 compatibility warnings, since I would assume the developer has already tested the add-on on 6 or we bumped it up ourselves.

Also, in some cases we could show old bump errors that prevent the new compatibility bump from happening and they could be false positives that were tested and dismissed by the author. Making the upgrade difficult every step of the way would be very annoying to these authors.

I think we should fix this bug.

Comment 4

[Matt Basta [:basta]]

Fixed:

https://github.com/mattbasta/amo-validator/commit/b9a299c4288dceab484a23598f646ffbcadf2f5d

The compatibility tests will now respect targetApplication minversion parameters. If the test applies to versions lower than the minVersion value for the application GUID, the tests will no longer apply.

Comment 5

[Matt Basta [:basta]]

Merged:

https://github.com/mozilla/amo-validator/commit/28b6a891a1643f779368ffd8178d153e5b2a8bcb

Comment 6

[krupa raj[:krupa]]

Bulk compatibility still shows warnings for firefox 6 compatibility. See https://addons.mozilla.org/en-US/developers/addon/pacman-176479/validation-result/37578

Comment 7

[Matt Basta [:basta]]

Kumar: This code needs to set both targetapp_minVersion as well as targetapp_maxVersion:

https://github.com/jbalogh/zamboni/blob/master/apps/devhub/tasks.py#L59

Do you want me to fix this?

Comment 8

[Kumar McMillan [:kumar]]

(In reply to comment #7)
> Do you want me to fix this?

Sure, go for it. I can review.

Comment 9

[Kumar McMillan [:kumar]]

(In reply to comment #8)
> (In reply to comment #7)
> > Do you want me to fix this?
> 
> Sure, go for it. I can review.

btw, this is actually where the fix needs to happen:
https://github.com/jbalogh/zamboni/blob/master/apps/zadmin/tasks.py#L68

You can use the max supported app version from res.file.version and use that as the override to targetapp_minVersion

Comment 10

[Kumar McMillan [:kumar]]

Fixed https://github.com/jbalogh/zamboni/commit/4143bdcb1179e5446d563531915ee3c9d1b477e2