393285 - Add OS and ABI for restricting the blocklist checks

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Robert Strong (they/them - no direct email)]

So we could match a plugin file name based on OS, etc.

Comment 1

[Robert Strong (they/them - no direct email)]

We also need to only provide the option to update a plugin when an update to the plugin is available (UI work for this to be done in bug 391714).

Comment 2

[Dave Townsend [:mossop]]

This would be needed to block plugins only on certain platforms

Comment 3

[Dave Townsend [:mossop]]

Rob, did you have any thoughts on how this would fit into the schema? I'm currently thinking of just an os and abi attribute on the pluginItem tag containing a comma separated list to match against.

I dont think we need regex's for this since these values should be well defined. We also might want to add it to the emItem as well to cover add-ons.

Comment 4

[Robert Strong (they/them - no direct email)]

That's what I was thinking on all points. I have some vague concerns about the implementation details and uniqueness but any issues like that should be sorted during the implementation. Note: the reason for ABI mainly came to mind do to x64, etc.

Comment 5

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This is a pretty simple fix, just ignore any emItem or pluginItem elements that match the os and/or xpcomabi check.

Comment 6

[Dave Townsend [:mossop]]

Attachment: unit tests rev 1

This is a set of unit tests for blocklisting. It is only really feasible to test the add-ons side of the blocklist service unfortunately. These testcases however won't work without either the next attachment or getting bug 319968 fixed (I'd prefer the latter)

Comment 7

[Dave Townsend [:mossop]]

Attachment: classinfo fixes

This removes any use of classinfo from the blocklist service, necessary for unit tests to work without bug 319968 being fixed.

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 284636 [details] [diff] [review]
classinfo fixes

I'm fine with waiting on bug 319968 or going with the classinfo fixes so +'ing and I'll let you decide if you want to wait.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 284635 [details] [diff] [review]
unit tests rev 1

Rob, can you just confirm you're ok with this.

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 284635 [details] [diff] [review]
unit tests rev 1

Looks fine and thanks Dave.

Comment 11

[Reed Loden [:reed]]

main patch:
Checking in toolkit/mozapps/extensions/src/nsBlocklistService.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsBlocklistService.js,v  <--  nsBlocklistService.js
new revision: 1.7; previous revision: 1.6
done

classinfo fixes:
Checking in toolkit/mozapps/extensions/src/nsBlocklistService.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsBlocklistService.js,v  <--  nsBlocklistService.js
new revision: 1.8; previous revision: 1.7
done

unit tests:
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug393285.xml,v
done
Checking in toolkit/mozapps/extensions/test/unit/data/test_bug393285.xml;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug393285.xml,v  <--  test_bug393285.xml
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug393285.js,v
done
Checking in toolkit/mozapps/extensions/test/unit/test_bug393285.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug393285.js,v  <--  test_bug393285.js
initial revision: 1.1
done

Comment 12

[Tony Chung [:tchung]]

Dave, are there any testcases here that QA can verify?  Or do the unit tests you added adequate for verification coverage?

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> Dave, are there any testcases here that QA can verify?  Or do the unit tests
> you added adequate for verification coverage?

The unit tests are I think adequate for the extensions side, Will provide a more thorough blocklist example in bug 391731 for plugins.

Comment 14

[Tony Chung [:tchung]]

Per the unit tests and the example in bug 391731, im marking this bug verified.