Ship an add-on blocklist by default

RESOLVED FIXED in seamonkey2.0b2

Status

defect
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: kairo, Assigned: kairo)

Tracking

Trunk
seamonkey2.0b2
Dependency tree / graph
Bug Flags:
blocking-seamonkey2.0b2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

10 years ago
Once AMO sends us a useful blocklist (see bug 505806), we should incorporate the current copy from there as a default blocklist in every SM installation, copying what Firefox does in http://mxr.mozilla.org/comm-central/source/mozilla/browser/app/Makefile.in#362 which installs blocklist.xml (and of course the package lists need to package that file).
Assignee

Comment 1

10 years ago
Nominating for b2 to get useful testing of the patch before final.
Flags: blocking-seamonkey2.0b2?

Updated

10 years ago
Flags: blocking-seamonkey2.0b2? → blocking-seamonkey2.0b2+
Assignee

Comment 2

10 years ago
This is the patch to add the current AMO blocklist to our builds, once bug 505806 gives us a better list on AMO, we should update the XML file in the tree, but the build integration is there with this patch.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #397466 - Flags: review?(neil)
Comment on attachment 397466 [details] [diff] [review]
add a blockist into our builds

>+libs:: $(srcdir)/blocklist.xml
>+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
This looks wrong, if compared to, say, the rules for the seamonkey script.

>+    <emItem id="langpack-vi-VN@firefox.mozilla.org">
>+      <versionRange minVersion="2.0" maxVersion="2.0"/>
>+    </emItem>
>+    <emItem id="support@daemon-tools.cc">
>+      <versionRange minVersion=" " maxVersion="1.0.0.5"/>
>+    </emItem>
These items bear no relevance to SeaMonkey, so I don't see the point of including them...

> bin/README
> bin/license.txt
>+bin/blocklist.xml
> bin/@MOZ_APP_NAME@-bin
> bin/@MOZ_APP_NAME@
> bin/run-mozilla.sh
> bin/application.ini
> bin/platform.ini
Hmm, our ordering here seems to be creation order, so I'd simply add blocklist.xml to the end of the list ;-)
Assignee

Comment 4

10 years ago
(In reply to comment #3)
> (From update of attachment 397466 [details] [diff] [review])
> >+libs:: $(srcdir)/blocklist.xml
> >+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
> This looks wrong, if compared to, say, the rules for the seamonkey script.

It's possible that other calls are missing the $(IFLAGS1) if the should have them. This is what e.g. Firefox uses and it makes sense, the flags set it to have 644 access flags on POSIX file systems.

> >+    <emItem id="langpack-vi-VN@firefox.mozilla.org">
> >+      <versionRange minVersion="2.0" maxVersion="2.0"/>
> >+    </emItem>
> >+    <emItem id="support@daemon-tools.cc">
> >+      <versionRange minVersion=" " maxVersion="1.0.0.5"/>
> >+    </emItem>
> These items bear no relevance to SeaMonkey, so I don't see the point of
> including them...

It's what AMO currently serves for us, I don't think we should ship anything else than what they give us, but it surely might make sense to remove those two one day, and we possibly should file a bug for that. It doesn't harm to block things that don't apply in any case, so I'd rather go with what they give us than diverge from it.

> > bin/README
> > bin/license.txt
> >+bin/blocklist.xml
> > bin/@MOZ_APP_NAME@-bin
> > bin/@MOZ_APP_NAME@
> > bin/run-mozilla.sh
> > bin/application.ini
> > bin/platform.ini
> Hmm, our ordering here seems to be creation order, so I'd simply add
> blocklist.xml to the end of the list ;-)

The order is somewhat up to taste here, yes, I just wanted to keep the executables, execute script, and relevant INIs together, I don't care much about everything else.
Assignee

Comment 5

10 years ago
Here's the patch with the .xml moved in the packages files and the list itself updated to the new state on AMO after bug 505806, now with the actually useful plugin blocks. I'm leaving the other items in to ship exactly what AMO has right now.
I'll file another bug on updating the other $(INSTALL) lines where needed, we should port bug 461322 over.
Attachment #397466 - Attachment is obsolete: true
Attachment #397836 - Flags: review?(neil)
Attachment #397466 - Flags: review?(neil)
Assignee

Comment 6

10 years ago
(In reply to comment #5)
> I'll file another bug on updating the other $(INSTALL) lines where needed, we
> should port bug 461322 over.

Bug 513900 now, btw.
Assignee

Comment 7

10 years ago
Oops, attached v1 again instead of the actual v2. Here's the right one.
Attachment #397836 - Attachment is obsolete: true
Attachment #397838 - Flags: review?(neil)
Attachment #397836 - Flags: review?(neil)
Assignee

Updated

10 years ago
Attachment #397836 - Attachment description: add blocklist in builds, v2 → add blocklist in builds, v1 (wrongly attached again)
Comment on attachment 397838 [details] [diff] [review]
add blocklist in builds, v2

>+libs:: $(srcdir)/blocklist.xml
>+	$(INSTALL) $(IFLAGS1) $^ $(DIST)/bin
I checked and there's no useful difference between $^ here and $< which we use in the rest of app/Makefile.in, but you can drop the $(srcdir)/ since that's the VPATH and will get picked up automatically; compare branding/Makefile.in:

>libs:: icons/gtk/seamonkey.png
>        $(INSTALL) $^ $(DIST)/bin/chrome/icons/default
> 
>install:: icons/gtk/seamonkey.png
>        $(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/chrome/icons/default
[Interesting that we do use $(IFLAGS1) this time!]

>\ No newline at end of file
Save as "XML document, complete" bit you :-(
Attachment #397838 - Flags: review?(neil) → review+
Assignee

Comment 9

10 years ago
(In reply to comment #8)
> >install:: icons/gtk/seamonkey.png
> >        $(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/chrome/icons/default
> [Interesting that we do use $(IFLAGS1) this time!]

Yes, the patch we need to port is about making things happening with $(INSTALL) and $(SYSINSTALL) equal.
Assignee

Comment 10

10 years ago
Pushed as http://hg.mozilla.org/comm-central/rev/02903d938d67
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Assignee

Updated

10 years ago
Blocks: 525465
You need to log in before you can comment on or make changes to this bug.