Closed Bug 591387 Opened 9 years ago Closed 9 years ago

Default theme shows as "incompatible" on the latest trunk nightly (also, version number wrapped with quotes)

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b5
Tracking Status
blocking2.0 --- beta5+

People

(Reporter: marcia, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

I noticed today when running Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4) Gecko/20100818 Firefox/4.0b4 that the default theme shows as incompatible. I don't recall this be the case yesterday.
Pushlog since yesterday:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75cc425bbaf0&tochange=e1d55bbd1d1d

Dave or Ben, could this be related to bug 586591?
Keywords: regression
Whiteboard: DUPEME
OS: Mac OS X → All
Hardware: x86 → All
(In reply to comment #1)
> Pushlog since yesterday:
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75cc425bbaf0&tochange=e1d55bbd1d1d
> 
> Dave or Ben, could this be related to bug 586591?

Can't imagine how it could be since she is running 4.0b4 which doesn't have that patch.
Marcia, can you attach a copy of extensions.sqlite from your profile folder.
I will attach the file in Comment 3 as well, but note I am running Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b5pre) Gecko/20100827 Minefield/4.0b5pre - I incorrectly pasted the wrong build ID in Comment 0.
Attached file Copy of extensions.sqlite file (obsolete) —
Attachment #469972 - Attachment is obsolete: true
Attachment #469973 - Attachment mime type: text/plain → application/octet-stream
Dave, something suspicious I see here is, that I'm not able to open the extensions.sqlite and addons.sqlite with the sqlite manager anymore. Other sqlite files can be opened without problems.
(In reply to comment #8)
> Dave, something suspicious I see here is, that I'm not able to open the
> extensions.sqlite and addons.sqlite with the sqlite manager anymore. Other
> sqlite files can be opened without problems.

Please scratch that. There was a process still running in the background.

I can see the same as Marcia only with todays build. The build from yesterday was fine.
The reason are the min and max version numbers:

<em:minVersion>"4.0b5pre"</em:minVersion>
<em:maxVersion>"4.0b5pre"</em:maxVersion> 

Somehow quotes have been added.
Good catch, looks like fallout from bug 581008
Blocks: 581008
blocking2.0: --- → beta5+
Attached patch patchSplinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #470040 - Flags: review?(ted.mielczarek)
Recommend you kick review to khuey, he's fast right now :)
Summary: Default theme shows as "incompatible" on the latest trunk nightly → Firefox version embraced in quotes (marks default add-ons as incompatible)
Duplicate of this bug: 591306
Reverting summary (for the most part) so it is easier for people to find since it originally stated the symptom.

btw: the cause is the minVersion and maxVersion being wrapped in quotes which isn't technically invalid but it is incompatible.
Summary: Firefox version embraced in quotes (marks default add-ons as incompatible) → Default theme shows as "incompatible" on the latest trunk nightly (also, version number wrapped with quotes)
Attachment #470040 - Flags: review?(ted.mielczarek) → review?(khuey)
FWIW, I suspect that this is fallout from the quoting changes that ted did to preprocessor.py.
(In reply to comment #13)
> Recommend you kick review to khuey, he's fast right now :)

Not sure that's the reputation I want :-P
Comment on attachment 470040 [details] [diff] [review]
patch

This patch should be a no-op (all it does is change the name of the variable from FIREFOX_VERSION to MOZ_UA_FIREFOX_VERSION so either there's something going on that I don't understand or it doesn't work.
Attachment #470040 - Flags: review?(khuey) → review-
Ok, so this patch works because the makefiles for all the extensions are adding -DFIREFOX_VERSION=$(FIREFOX_VERSION) to the DEFINES variable.  This patch stops adding FIREFOX_VERSION to the DEFINES in configure, which lets those makefiles pick up the work.

What you should do is take the current patch, s/MOZ_UA_FIREFOX_VERSION/MOZ_APP_UA_FIREFOX_VERSION/ (kind of messy, but at least consistent), and then AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION) (with no quotes at all).  Then remove http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/extensions/%7B972ce4c6-7e08-4474-a285-3208198ce6fd%7D/Makefile.in#46 http://mxr.mozilla.org/mozilla-central/source/extensions/widgetutils/Makefile.in#19 and http://mxr.mozilla.org/mozilla-central/source/extensions/metrics/Makefile.in#66
Actually, on second though, I think MOZ_UA_FIREFOX_VERSION is preferable since that has nothing to do with the actual app we're building.
(In reply to comment #21)
> So what remains is this:
> 
> > AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION) (with no quotes at all). 
> > Then remove
> > http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/extensions/%7B972ce4c6-7e08-4474-a285-3208198ce6fd%7D/Makefile.in#46
> > http://mxr.mozilla.org/mozilla-central/source/extensions/widgetutils/Makefile.in#19
> > and
> > http://mxr.mozilla.org/mozilla-central/source/extensions/metrics/Makefile.in#66
> 
> ... which is orthogonal to this bug?

Yeah, we can do that in a followup.
Comment on attachment 470040 [details] [diff] [review]
patch

This is fine for the moment, I'll file that followup.
Attachment #470040 - Flags: review- → review+
http://hg.mozilla.org/mozilla-central/rev/7e70423b8749
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Comment on attachment 470040 [details] [diff] [review]
patch

>diff -r 256514ea6d14 config/autoconf.mk.in
>+MOZ_UA_FIREFOX_VERSION = @FIREFOX_VERSION@

I'm pretty sure the right-hand side of this should be @MOZ_UA_FIREFOX_VERSION@ instead, as that's what the AC_SUBST is using.
(In reply to comment #25)
> Comment on attachment 470040 [details] [diff] [review]
> patch
> 
> >diff -r 256514ea6d14 config/autoconf.mk.in
> >+MOZ_UA_FIREFOX_VERSION = @FIREFOX_VERSION@
> 
> I'm pretty sure the right-hand side of this should be @MOZ_UA_FIREFOX_VERSION@
> instead, as that's what the AC_SUBST is using.

It's more complicated than that, but it doesn't matter since nothing is actually using that in a makefile.  This is fairly hacky, I'm going to clean it up in bug 591611.
Should be fairly simple to add a mochitest to verify that the default theme's target application minVersion / maxVersion equals the current application's version to prevent this from slipping by in the future
Flags: in-testsuite?
Duplicate of this bug: 591783
Duplicate of this bug: 591783
This doesn't seem to be fixed for the langpacks, at least.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #30)
> This doesn't seem to be fixed for the langpacks, at least.

Please file a bug.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
There have been two bugs resolved as DUPE of this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I reopened that bug. This one is fixed.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b5pre) Gecko/20100829 Firefox/4.0b5pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.