Ship an add-on blocklist by default

RESOLVED FIXED in seamonkey2.0b2

Status

SeaMonkey
Build Config
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Robert Kaiser, Assigned: Robert Kaiser)

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

8 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

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

Updated

8 years ago
Flags: blocking-seamonkey2.0b2? → blocking-seamonkey2.0b2+
(Assignee)

Comment 2

8 years ago
Created attachment 397466 [details] [diff] [review]
add a blockist into our builds

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 3

8 years ago
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

8 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

8 years ago
Created attachment 397836 [details] [diff] [review]
add blocklist in builds, v1 (wrongly attached again)

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

8 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

8 years ago
Created attachment 397838 [details] [diff] [review]
add blocklist in builds, v2

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

8 years ago
Attachment #397836 - Attachment description: add blocklist in builds, v2 → add blocklist in builds, v1 (wrongly attached again)

Comment 8

8 years ago
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

8 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

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

Updated

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