Closed Bug 654720 Opened 9 years ago Closed 9 years ago

Could not read extensions chrome manifest file (972ce4c6-7e08-4474-a285-3208198ce6fd = default theme), SeaMonkey workaround

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey2.1final

People

(Reporter: sgautherie, Assigned: Nomis101)

References

Details

Attachments

(1 file, 3 obsolete files)

Port (TB) bug 654672, as there is no progress in (Toolkit) bug 586610.

Bug 586610 comment 14:
{
Nomis101      2011-02-21 14:28:49 PST

I can do the same for suite as well.
}
That would be nice :-)
Attached patch Workaround for SM (obsolete) — Splinter Review
I haven't tested this one!
I have no idea about the SM review policy, so please choose a reviewer for this patch as you think.
Comment on attachment 530057 [details] [diff] [review]
Workaround for SM

Review of attachment 530057 [details] [diff] [review]:

Untested, but f+ with nits fixed.

::: suite/installer/removed-files.in
@@ +607,5 @@
   extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/
   extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/icon.png
   extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/install.rdf
   extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/preview.png
+  extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/chrome.manifest

Good point! (SM has an '#ifdef MOZ_OMNIJAR' that TB doesn't have.)

::: suite/installer/package-manifest.in
@@ +446,5 @@
 #else
 @BINPATH@/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/icon.png
 @BINPATH@/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/install.rdf
 @BINPATH@/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/preview.png
+@BINPATH@/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/chrome.manifest

Nit: in all files, please insert in alphabetical order.

::: suite/themes/classic/Makefile.in
@@ +50,5 @@
 
 FILES = \
 	$(srcdir)/preview.png \
 	$(srcdir)/icon.png \
+       $(srcdir)/chrome.manifest \

Nit: misaligned. (and modern too.)
Attachment #530057 - Flags: review?(neil)
Attachment #530057 - Flags: feedback+
Attached patch SM Workaround (nits fixed) (obsolete) — Splinter Review
So, now everything should be in alphabetic order. :-)

The misalignment thing is strange, it was not misaligned in the file, but in the patch. Now it should be properly aligned, I hope...
Attachment #530057 - Attachment is obsolete: true
Attachment #530057 - Flags: review?(neil)
Attachment #530095 - Flags: review?(neil)
Comment on attachment 530095 [details] [diff] [review]
SM Workaround (nits fixed)

Review of attachment 530095 [details] [diff] [review]:

Untested, but f+ with (1st) nits fixed.

::: suite/themes/classic/Makefile.in
@@ +48,5 @@
 
 DEFINES += -DSEAMONKEY_VERSION=$(SEAMONKEY_VERSION)
 
 FILES = \
+       $(srcdir)/chrome.manifest \

Nit: patch should use a tab, not spaces. (modern too.)

@@ +49,5 @@
 DEFINES += -DSEAMONKEY_VERSION=$(SEAMONKEY_VERSION)
 
 FILES = \
+       $(srcdir)/chrome.manifest \
+	$(srcdir)/icon.png \

Nit: I don't mind touching existing line :-) But I'm not sure whether reviewer will or not: you'll see.
Attachment #530095 - Flags: feedback+
I think I see this as: "Could not find jar manifest entry 'chrome.manifest'."
Duplicate of this bug: 595552
(In reply to comment #6)
> I think I see this as: "Could not find jar manifest entry 'chrome.manifest'."

[Mozilla/5.0 (Windows NT 5.0; rv:6.0a1) Gecko/20110430 Firefox/6.0a1 SeaMonkey/2.2a1pre] (nightly)

Yeah, error message has probably changed in the meantime.
Comment on attachment 530095 [details] [diff] [review]
SM Workaround (nits fixed)

Doesn't Modern already provide a chrome.manifest?
(In reply to comment #9)
> Doesn't Modern already provide a chrome.manifest?

Right! SM 2.x modern@themes.mozilla.org.xpi does contain a chrome.manifest.
Iirc, I had checked that only Classic (= default) theme was affected when I first noticed this (Core) bug.
Comment on attachment 530095 [details] [diff] [review]
SM Workaround (nits fixed)

> FILES = \
>+       $(srcdir)/chrome.manifest \
>+	$(srcdir)/icon.png \
> 	$(srcdir)/preview.png \
>-	$(srcdir)/icon.png \
> 	$(NULL)
Nit: Please use a tab for consistency with the other lines.

>diff --git a/suite/themes/modern/Makefile.in b/suite/themes/modern/Makefile.in
>diff --git a/suite/themes/modern/chrome.manifest b/suite/themes/modern/chrome.manifest
Please remove the Modern changes. r=me with that fixed.
Attachment #530095 - Flags: review?(neil) → review+
Caution: Given that the TB guys backed out their solution (see bug 654672 comment 3) and investigation is pending, this should at least not land on branch yet.
Yes, the TB patch was backed out, because of a problem with mac builds. I try to fix the TB patch and after that I make a new SM one.
Attached patch New patch (obsolete) — Splinter Review
In this patch, I used tabs, I removed the modern change and I included the fix for the TB bustage.
Attachment #530095 - Attachment is obsolete: true
(In reply to comment #14)
> Created attachment 530688 [details] [diff] [review] [review]
> New patch
> 
> In this patch, I used tabs, I removed the modern change and I included the
> fix for the TB bustage.

As stefanh noted on IRC, there's a copy/paste mistake in the last few lines:

+++ b/mail/themes/qute/mail/chrome.manifest
      ^^^^
Assignee: nobody → Nomis101
Status: NEW → ASSIGNED
(Actually the whole line is referring to TB, not just what I highlighted.)
Oops!
Attachment #530688 - Attachment is obsolete: true
Attachment #530727 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.2pre) Gecko/20110507 SeaMonkey/2.1pre, no more manifest-related message on startup; thanks Nomis!
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.