Extend blocklist to support plugins

VERIFIED FIXED in 3.4

Status

defect
VERIFIED FIXED
12 years ago
3 years ago

People

(Reporter: rstrong, Assigned: morgamic)

Tracking

unspecified
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Details

(Whiteboard: [has patch][has reviews])

Attachments

(2 attachments, 1 obsolete attachment)

mwu has added support for blocklisting plugins in bug 330511.
Flags: blocking-firefox3?

Comment 1

12 years ago
Bug 271559 actually adds the code that reads and uses plugins blocklisting information from blocklist.xml.
Depends on: 271559
Assignee

Comment 2

12 years ago
Is there anything about plugins that extends past the scope of the current blocklist.xml schema?  See:
http://wiki.mozilla.org/Extension_Blocklisting:Code_Design

Comment 3

12 years ago
The plugin blocklisting update to the blocklist.xml schema are fairly simple. Here's an example:

<blocklist>
  <pluginItems>
    <pluginItem>
      <!-- All match tags must match a plugin to blocklist a plugin -->
      <match name="name" exp="some plugin"/>
      <match name="description" exp="1[.]2[.]3"/>
      </pluginItem>
  </pluginItems>
</blocklist> 

The name attribute in the match tag can match any string attribute defined in http://lxr.mozilla.org/mozilla/source/modules/plugin/base/public/nsIPluginTag.idl . The exp attribute contains a regular expression which is used to match an attribute in the plugin tag. If all the match tags in a pluginitem match, the plugin is blocklisted.
Flags: blocking-firefox3? → blocking-firefox3+
hi, just wondering what the status and timeline for this feature.  is there plans to target beta 3?  
thats what i thought too.  when i verified that bug, at least the notification part was working.   If this is really fixed, can we get this bug properly commented and status changed?
Assignee

Comment 7

12 years ago
TODO:
* update blocklist schema wiki page (see comment #2)
* create new table for pluginitems (kind of like blitems, but blpluginitems instead)
* add logic in script to pull from plugin table
* add logic in script to iterate over results and display plugin list in the XML

ETA is Feb 7-ish.
Assignee: nobody → morgamic
Assignee

Updated

12 years ago
Status: NEW → ASSIGNED
Assignee

Comment 8

12 years ago
Stephen/Tony -- plugins don't have GUIDs, so the name/regex matching in pluginItem entries is a fix for that.  Since they followed a different install method for a pretty long time before we centered on the method used for today's extensions, they are a special case and aren't covered by the existing schema.
Assignee

Comment 9

12 years ago
mwu -- so just description, filename and name, right?
Assignee

Comment 10

12 years ago
One more quesiton -- if I have a <pluginItem> entry is it possible to define an application range for it, or is blocklisting of a plugin application-agnostic?

For extensions it was possible to define this:
    <emItem id="item_4@domain">
      <versionRange>
        <targetApplication>
          <versionRange minVersion="1.5" maxVersion="1.5.*"/>
        </targetApplication>
      </versionRange>
    </emItem>

I'm guessing it's app-agnostic since they are pure binaries, but want to confirm.
Assignee

Comment 11

12 years ago
Answers:
#9: yes, just those
#10: yes, app-agnostic

Wanted to make sure.  So we'll move forward with those assumptions.  I updated the wiki to reflect this:
http://wiki.mozilla.org/Extension_Blocklisting:Code_Design#Blocklist_syntax
Assignee

Updated

11 years ago
Target Milestone: --- → 3.2
Assignee

Comment 12

11 years ago
Will add tests tomorrow -- Fred can you take a look at the schema and basics?
Attachment #308590 - Flags: review?(fwenzel)
Comment on attachment 308590 [details] [diff] [review]
first stab at schema and blocklist.php patch

In general, the patch looks good.

But please use a MySQL index that spans the two columns we are querying here together, not each separately.

(Also, if you want to remove a typo, line 240 says "errer" for "error").
Attachment #308590 - Flags: review?(fwenzel) → review-
Assignee

Updated

11 years ago
Blocks: 421993
Assignee

Comment 14

11 years ago
K, fixed typo -- pretty sure that a multicolumn index won't make any difference, but will check...
Target Milestone: 3.2 → 3.4
Blocks: 424158
Whiteboard: [needs status update]
Mike, ETA on getting this into production?
Assignee

Comment 16

11 years ago
Just gotta write test so tomorrow looks peachy.
Assignee

Comment 17

11 years ago
Tony - will work with you to stage it first as well.
thanks, i just need to know which plugin we plan to blocklist, version, and where to point my settings to.   I'll have time to test this after we ship beta 5
Blocks: 426780
Assignee

Comment 19

11 years ago
Actually I need to know what we're blocklisting as far as plugins... so I can create a record for it?
Assignee

Comment 20

11 years ago
To test, have to set SERVICE_URL and make sure you have TEST_DB_* setup correctly in your config (which you should already!!!) >:P
Attachment #308590 - Attachment is obsolete: true
Attachment #313592 - Flags: review?(fwenzel)
Comment on attachment 313592 [details] [diff] [review]
v2, proper index, tests and test data, fixed typo

Passes the test suite, looks good. Good job.
Attachment #313592 - Flags: review?(fwenzel) → review+
Assignee

Comment 22

11 years ago
Checked in on trunk.  Tony, let's meet on Friday to test a random plugin -- I'll ping ya.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: push-needed
Resolution: --- → FIXED
Whiteboard: [needs status update]
Assignee

Comment 23

11 years ago
We need to add support for version _ranges_ in the app (Firefox can't parse them as with blitems/blapps).  So reopening until version range checking is added.  For now we can test with a specific version, though.  Won't push until range support is added.
Status: RESOLVED → REOPENED
Keywords: push-needed
Resolution: FIXED → ---
Assignee

Updated

11 years ago
Status: REOPENED → ASSIGNED
Assignee

Comment 24

11 years ago
ETA on this is Saturday.
Whiteboard: [has patch][has reviews]
Assignee

Comment 25

11 years ago
S/Saturday/Monday/ -- started the patch but wasn't able to finish this weekend.  Patch later this PM.
Assignee

Comment 26

11 years ago
Patch on top of v2 which adds support for version ranges, tests included.
Attachment #314194 - Flags: review?(fwenzel)
Comment on attachment 314194 [details] [diff] [review]
v3, adding support for version ranges

WFM. New tests look good and pass as well.

Not something to r- it for, but maybe you want to avoid the relative path in the require_once since it relies on the CWD and make it relative to dirname(__FILE__) instead?
Attachment #314194 - Flags: review?(fwenzel) → review+
Assignee

Comment 28

11 years ago
Yeah, added dirname(__FILE__) thanks Fred.  Checked in on r12064.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Assignee

Updated

11 years ago
Keywords: push-needed
Assignee

Updated

11 years ago
Keywords: push-needed
Verified FIXED; I tested this today using Tony's Litmus testcases, and they all passed (Java, QuickTime, etc.), with rc1.
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.