Create add-ons compatibility dashboard for Thunderbird

VERIFIED FIXED in 5.5

Status

addons.mozilla.org Graveyard
Compatibility Tools
P3
normal
VERIFIED FIXED
10 years ago
2 years ago

People

(Reporter: gkw, Assigned: cesar)

Tracking

unspecified
Dependency tree / graph

Details

(Whiteboard: [no l10n impact][tb3needs], URL)

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
+++ This bug was initially created as a clone of Bug #460309 +++

Thunderbird should its own form of the add-ons compatibility dashboard. The Firefox equivalent is here: https://addons.mozilla.org/en-US/firefox/compatibility

Updated

10 years ago
Assignee: nobody → rebron
(Reporter)

Updated

10 years ago
Version: 3.2 → unspecified
Flagging as tb3 needs so this gets onto the appropriate trackers (although I think we'll be wanting it before then).
Whiteboard: [tb3needs]

Comment 2

9 years ago
FYI, bug 509501 requests this for SeaMonkey 2.0 as well.

Updated

9 years ago
Whiteboard: [tb3needs] → [tb3needs][no l10n impact]

Updated

9 years ago
Whiteboard: [tb3needs][no l10n impact] → [no l10n impact][tb3needs]
(Assignee)

Comment 3

9 years ago
A compatibility dashboard for Thunderbird already exists (https://addons.mozilla.org/en-US/thunderbird/compatibility) but it just seems to be reporting Firefox stats rather than Thunderbird stats
(In reply to comment #3)
> A compatibility dashboard for Thunderbird already exists
> (https://addons.mozilla.org/en-US/thunderbird/compatibility) but it just seems
> to be reporting Firefox stats rather than Thunderbird stats

Yes, that's because it was made generic, but not generic enough :-( which is why this bug exists.
This could probably be tracked by one bug.  Wil - how much work would it be to modify the compat dashboard to support multiple apps?
It's a good bit of work, mainly because the backend report generator is setup to only pull data for Firefox. The frontend changes won't be as hard.

http://svn.mozilla.org/addons/trunk/bin/compatibility_report.php

Comment 7

9 years ago
Hrm, that's bad... Both Thunderbird and SeaMonkey would really love to get something to work there, any chance that work could be done? Any ETA for that?

I start to think we're too late for this for both Thunderbird 3.0 and SeaMonkey 2.0 anyhow, but it would be nice to have it for the next versions that come up a few months later.

Any chance to at least e.g. get a mail out to everyone who has add-ons that are compatible with Thunderbird or SeaMonkey so we can at least tell them and ask for getting their work compatible with our new releases?
(Assignee)

Comment 8

9 years ago
If the assignee is no longer working on this, which is likely the case, I volunteer to take it. I've started modifying compatibility_report, but it's completely untested since the stock data in AMO isn't sufficient enough to test this (there are no update_counts data, for one). Need to figure out what data to insert into what table.
reassigning->Cesar
Assignee: rebron → cdolivei.bugzilla
(Assignee)

Comment 10

9 years ago
Created attachment 409027 [details] [diff] [review]
compatibility report changes

Here is the progress on compatibility_report. It's not yet review ready, but it should be enough for AMO to get Thunderbird stats. You can get stats by running |php compatibility_report.php tb|. Front-end stuff is still WIP. 

Additionally, we now have to filter on application as well. Something not done in this patch. Some of the global variables might have to move around into better places.

Comment 11

9 years ago
Cesar, could you please add the same for SeaMonkey at the same time? From all I see in your patch, that should be pretty straight forward to do there...
(Assignee)

Comment 12

9 years ago
(In reply to comment #11)
> Cesar, could you please add the same for SeaMonkey at the same time? From all I
> see in your patch, that should be pretty straight forward to do there...

It would be relatively straightforward. However, I'm going to do that separately in bug 509501
(Assignee)

Comment 13

9 years ago
Created attachment 409644 [details] [diff] [review]
first attempt

This makes the appropriate changes to the compatibility_report and backend/frontend dashboard code. You can run the thunderbird compatibility_report generator by running "php compatibility_report.php tb".

This is almost completely working, the only thing missing are the Thunderbird wordmarks.
Attachment #409027 - Attachment is obsolete: true
Attachment #409644 - Flags: review?(fligtar)

Comment 14

9 years ago
(In reply to comment #12)
> (In reply to comment #11)
> > Cesar, could you please add the same for SeaMonkey at the same time? From all I
> > see in your patch, that should be pretty straight forward to do there...
> 
> It would be relatively straightforward. However, I'm going to do that
> separately in bug 509501

OK, thanks for looking into that - as SeaMonkey 2.0 has shipped already, we're in a different situation there in any case (I'll leave it to others to judge if it's better or worse).
Thanks for working on this Cesar. I will try to review it by code freeze tomorrow, but I have a lot of stuff on my plate and may not be able to. After glancing through it just now, a couple comments:

* Rather than have to set up a bunch of cron jobs for each application, let's just go through all of the supported apps when we run the one cron.

* I'm pretty sure we have a list of "short names" for the applications somewhere already and shouldn't need to add that new array to constants.php
(Assignee)

Comment 16

9 years ago
Created attachment 410130 [details]
goes into /webroot/img/wordmarks/
(Assignee)

Comment 17

9 years ago
Created attachment 410135 [details]
Goes in /webroot/img/wordmarks

Thanks for the feedback Justin. It was unlikely to get into this release anyways, since the patch would have to be approved on the first review.
Component: Public Pages → Compatibility Tools
QA Contact: web-ui → compatibility
Attachment #409644 - Flags: review?(fligtar) → review-
Comment on attachment 409644 [details] [diff] [review]
first attempt

Finally got a chance to review this. Great work, but we should make a few changes.

* as mentioned above, we should loop through the applications that have compatibility report versions listed and do them all at once instead of adding a cron job for each app

* Your patch references $compat_default_version to get a per-app default version, but it's not defined anywhere so doesn't actually work.

* When pulling app guids from within Cake, please use the available methods in the Application model.

Also, there's a bug in the entire dashboard that is worsened by adding support for other applications, and that is that add-on usage and ordering takes into account all of the add-on's update pings, not just the update pings for that application. For Firefox this isn't a huge deal, but for Thunderbird, it shows Adblock Plus as taking up a huge chunk of all Thunderbird add-on usage, which isn't the case. You don't need to fix that in this patch as it's going to require some significant changes, but just something to be aware of.
Priority: -- → P3
Target Milestone: --- → 5.5
Cesar, any chance you can get an updated patch up soon?  We're desperate for this =).  Let me know if I can help.
(Assignee)

Comment 20

9 years ago
(In reply to comment #19)
> Cesar, any chance you can get an updated patch up soon?  We're desperate for
> this =).  Let me know if I can help.

I should be able to get it in 5.5. Unfortunately, my move to Regina from Toronto couldn't include my remora box. So I have to re-setup a remora instance on my laptop. But once that is done, I will be able to update this patch.
(Assignee)

Comment 21

9 years ago
Created attachment 418577 [details] [diff] [review]
Fixed and updated

Sorry about the compat_default_version variable missing. I likely put it in config.php without copying it to .default.

This patch is newer, and arguably better. I get rid of a lot of duplicate hard-coded data in favour of what is in the db.

This patches was made with -w being passed, to ignore whitespace changes (I did a foreach loop in the compatibility_report. Just to reduce the size of the patch). So don't worry if code isn't tabbed correctly, the real patch correctly spaces everything.

I assigned bug 525416 to myself, which will make the compatibility report actually useful for other applications.
Attachment #409644 - Attachment is obsolete: true
Attachment #418577 - Flags: review?(fligtar)
Comment on attachment 418577 [details] [diff] [review]
Fixed and updated

I started out reviewing bin/compatibility_report.php but I can't seem to get it to work. When I run it it creates the serialized files for each app and version, but it doesn't have the data. The values are all 0.

Additionally, a code comment for just that file:

> /**
>+ * Retrieve various information from the database, to avoid having
>+ * duplicate data
>+ */
>+$app_qry = $db->read("
>+            SELECT a.id, a.guid, t.localized_string AS 'name', r.localized_string AS 'shortname'
>+            FROM applications a
>+            LEFT JOIN translations t ON (a.name = t.id)
>+            LEFT JOIN translations r ON (a.shortname = r.id)
>+            WHERE supported = 1");

There should be a locale='en-US' where clause in there.
Attachment #418577 - Flags: review?(fligtar) → review-
(Assignee)

Comment 23

9 years ago
(In reply to comment #22)
> (From update of attachment 418577 [details] [diff] [review])
> I started out reviewing bin/compatibility_report.php but I can't seem to get it
> to work. When I run it it creates the serialized files for each app and
> version, but it doesn't have the data. The values are all 0.
> 

Is your update_counts table empty? The remora test data doesn't actually include any data there. I had to figure that out based on the code that uses the data. I may have made a wrong conclusion, so it wouldn't work. Does it work without the patch?

I'll update the locale where clause.
I reverted the patch to see if it worked before it, and it worked fine, so that's why I assumed it was something in the patch and not my setup.
(Assignee)

Comment 25

9 years ago
(In reply to comment #24)
> I reverted the patch to see if it worked before it, and it worked fine, so
> that's why I assumed it was something in the patch and not my setup.

It's strange that I can't reproduce this. By any chance, did you try adding the locale = 'en-US' change to the DB query and seeing if that fixes it? Turns out, remora test data only includes english names. So if it was grabbing some translation of Firefox, I likely wouldn't catch it.
Comment on attachment 418577 [details] [diff] [review]
Fixed and updated

Adding the locale where clause fixed the file generation problem, and fixing the typo below fixed the report display problems.

>Index: site/app/config/config.php.default
>...
>+global $compat_default_version;
>+$compat_default_veresion = array(

This was a fun typo to debug.

>Index: site/app/controllers/compatibility_controller.php
>...
>+    function dashboard($version = null, $view = '') {
>+        global $compatibility_versions, $app_shortnames, $compat_default_version;
>+        if ($version == null) {
>+            global $compat_default_version;

$compat_default_version is globaled twice.

I'll r+ a new patch with those 3 fixes. We should also get the 2 Thunderbird wordmarks needed.
Justin,

what are the wordmarks you need?  Do you have an example of the old ones?
(Assignee)

Comment 28

9 years ago
(In reply to comment #27)
> what are the wordmarks you need?  Do you have an example of the old ones?

We need Thunderbird equivalents of https://addons.mozilla.org/img/wordmarks/firefox-3.6.png and https://addons.mozilla.org/img/wordmarks/firefox-3.6_small.png used in https://addons.mozilla.org/firefox/compatibility

I attached some, but it doesn't have a version number. Or might not be good enough quality.

I will upload the new patch tonight. I gather it doesn't need another review as long as those three fixes are in place.
Code freeze is tonight.  Cesar, do you have a patch for this?
Created attachment 420187 [details]
Thunderbird 3 .png large
Created attachment 420189 [details]
Thunderbird 3 .png small

will these work?  I'm not sure what you need for the last page, if you just an icon or the whole thing "Addons for [logo] Thunderbird"
(Assignee)

Comment 32

9 years ago
(In reply to comment #31)
> Created an attachment (id=420189) [details]
> Thunderbird 3 .png small
> 
> will these work?  I'm not sure what you need for the last page, if you just an
> icon or the whole thing "Addons for [logo] Thunderbird"

Thanks Rafael, but the small and large pngs are the same. I think you uploaded the same png twice. We have a addons for thunderbird already (https://addons.mozilla.org/en-US/thunderbird/compatibility). We just needed those two watermarks

Updated

9 years ago
Attachment #420187 - Attachment is obsolete: true
Created attachment 420210 [details]
Thunderbird 3 .png large
Created attachment 420211 [details]
Thunderbird Logo .png

Cesar, can you use this logo for this page: https://addons.mozilla.org/en-US/thunderbird/compatibility
(Assignee)

Comment 35

9 years ago
Thanks fligtar and everyone to help make this happen. In r59070. The new logo clashes with the old one, but bug 536066 has been already been filed on that. We still need bug 525416 to be fixed before the data will be useful to Thunderbird (right now, extensions with both firefox and thunderbird users will count both products as the total user base, instead of per application).

Wil: can you assign someone to fix the config.php? They can look in config.php.default as $compat_default_version global variable has been added, and compatibility_versions and version_aliases has been modified. So please copy those over.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Keywords: push-needed
(In reply to comment #35)
> Wil: can you assign someone to fix the config.php? They can look in
> config.php.default as $compat_default_version global variable has been added,
> and compatibility_versions and version_aliases has been modified. So please
> copy those over.

oremj says this is done and will sync out in a few minutes
Mark, Gary: can you guys check out https://preview.addons.mozilla.org/en-US/thunderbird/compatibility and let us know if you find bugs?

I don't think we can yet test this fully, though -- the "Detailed Report" link, available from the "View Compatibility Report" link doesn't list any add-ons -- probably just missing data on preview.
(Assignee)

Comment 38

9 years ago
That should be fixed the next time compatibility_report.php is run. Hopefully that does run on preview :)
This is a great step forward, thanks!  Is there an ETA on getting bug 525416 fixed?  (assuming that's what will discard things like flashblock from our default dashboard view)?
Issues I've noticed:

- https://preview.addons.mozilla.org/en-US/firefox/compatibility

This displays a blank page.

https://preview.addons.mozilla.org/en-US/thunderbird/compatibility

- On the detailed compatibility report, "3.0b4" and "3.0pre" are indicated as compatible with the final version (dark green), rather than compatible with beta version.

- As davida said getting the right metrics for most-used extensions would be good.

The following links from the developer section are not found:

https://developer.mozilla.org/En/Updating_extensions_for_Thunderbird_3.0
https://developer.mozilla.org/en/Thunderbird_3.0_for_developers

The latter should actually be https://developer.mozilla.org/En/Thunderbird_3_for_developers however as I think you're probably automatically generating this we can just set up a redirect on devmo (hence cc'ing our docs person).

The former doesn't exist for Thunderbird 3 - we should fix that.

Updated

9 years ago
Depends on: 525416

Updated

9 years ago
Blocks: 509501
> - https://preview.addons.mozilla.org/en-US/firefox/compatibility
> 
> This displays a blank page.
Fixed out of memory errors in r59099

Comment 43

9 years ago
comment 34 is not fixed.

screenshot: http://screencast.com/t/MWJmOWNmO

URL:https://addons.mozilla.org/en-US/thunderbird/compatibility

Reopening...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 44

9 years ago
please ignore comment 43.this is fixed.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED

Comment 45

9 years ago
verified
Status: RESOLVED → VERIFIED
(In reply to comment #40)
> https://preview.addons.mozilla.org/en-US/thunderbird/compatibility
> 
> - On the detailed compatibility report, "3.0b4" and "3.0pre" are indicated as
> compatible with the final version (dark green), rather than compatible with
> beta version.

I'm not convinced about the verified status of this bug - the above item hasn't been fixed yet.

Comment 47

9 years ago
reopening as per comment #46.Thanks Mark!
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 48

9 years ago
Your right. I couldn't reproduce this on my machine, as 3.0pre would appear as beta. But I finally found out why, and the answer turned out to be quite simple : preview doesn't have an appversion of 3.0.* for thunderbird

https://preview.addons.mozilla.org/en-US/thunderbird/pages/appversions

The most recent entry is 3.0 entry is 3.0pre, which would make it the latest release. I guess b4 is also considered the latest. As is 3.1a1pre... This stopped making a whole lot of sense. But I think if 3.0 and 3.0.* were valid app versions, it should work.
(Assignee)

Comment 49

9 years ago
Wil was nice enough to add those versions into preview. The compatibility script was ran, and it the dashboard is now giving the correct colours. Mark or Krupa can you please confirm?
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to comment #49)
> Wil was nice enough to add those versions into preview. The compatibility
> script was ran, and it the dashboard is now giving the correct colours. Mark or
> Krupa can you please confirm?

Yes, that seems fine now. Thanks.
Keywords: push-needed
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.