Metrics extension compatibility with Google Toolbar

RESOLVED FIXED

Status

Toolkit Graveyard
Data Collection/Metrics
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: Alex Polvi, Assigned: Pete Collins (MDG))

Tracking

Details

(Whiteboard: [npotb?], URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Google Toolbar user similar methods and preferences as the metrics extension that lives in trunk. It will have to be possible to run both side by side, so ensuring compatibility is a must.
(Assignee)

Comment 1

11 years ago
Yes google toolbar is using the same prefs which is bad it really should be: "extensions.google.metrics.*"

I would recommend we change mozilla metrics prefs to something like:  "extensions.mozilla.metrics.*"

We might want to consider changing the uuid of the interfaces.

The contract id "@mozilla.org/metrics/service;1" is fine ... 
Assignee: nobody → pete
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

11 years ago
Created attachment 290588 [details] [diff] [review]
proposed patch

proposed patch to avoid potential conflicts w/ google toolbar.

Jan, can you please review when you get a chance.
(Assignee)

Comment 3

11 years ago
Ok, I just checked and googletoolbar is using the same contractID

  @mozilla.org/metrics/service;1

So maybe we should change the contractID to

  @mozilla.org/metrics/service;2

or maybe:

  @mozilla.org/extensions/metrics/service;1

(Assignee)

Comment 4

11 years ago
Created attachment 290704 [details] [diff] [review]
patch with revised contractID's

Changing out the uuids and contactIDs is probably not necessary but I think prudent as to aviod potential conflicts w/ google toolbar extension ...
(Assignee)

Updated

11 years ago
Attachment #290588 - Attachment is obsolete: true

Updated

11 years ago
Attachment #290704 - Flags: review+
(Assignee)

Comment 5

11 years ago
checked in yesterday. Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 6

10 years ago
Google Toolbar for Firefox 3.0 has issue with changed interface ids for the same interface names. For some time we plan to distribute extension compatible with Firefox 2 and 3 (same .xpi bundle) so that our users won't need to update on Firefox upgrade. For this we need .xpt files for both metrics extensions to be loaded. We were unable to find a way to hide one .xpt file or another. We get messages complaining about same interface name registered with different iids and one of versions is unable to find interfaces. 

Reverting iids back saves the problem. We believe keeping iids is safe as there are no interface changes in metrics.
 
What created conflict in the previous version was toolbar including metrics code as is. So far we don't feel a need in the separate metrics implementation. It would be really useful for us to make metrics work for both products. For example, we can avoid modification of upload uri and let metrics extension collect data when it's set up. We can also check metrics presence and don't load our copy unless it's necessary.
(Assignee)

Comment 7

10 years ago
We changed the contract ID's, interface ID's, and prefs so that there wont be any unanticipated collisions if users have the two implementations installed simultaneously. 

I was under the impression that the google implementation was a separate implementation that was *based* on metrics code.

If you are using a vanilla build of metrics, then can't you just spin a new build of the component and include it in your toolbar releases for FF3?

Reverting back the iids is fine by me if that solves the problem. But the current builds of metrics for FF3 is already released.


Comment 8

10 years ago
Created attachment 332544 [details] [diff] [review]
Rename public interfaces.

In order to have single .xpi that provides the metrics component compatible with both Firefox 2 and 3 we have to avoid conflict of .xpt's of different versions of metrics. Two ways are possible: make interfaces (names and iid's) identical or make them completely different. The second way looks more robust. The proposed patch renames the public interfaces nsIMetricsService, nsIMetricsCollector and nsIMetricsEventItem.

Comment 9

10 years ago
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

Pete, could you review this patch?
Attachment #332544 - Flags: review?(pete)
(Assignee)

Comment 10

10 years ago
The iid's of the interfaces don't matter. The conflict arises only when there is another extension that wants to use the metrics interfaces w/ a separate implementation for those interfaces and that implementation is carrying the same named interfaces w/ different iid's.

I think the Google folks want to be able to do everything in a single xpi. 

I'm not sure how you are gong to have binary compatibility between FF2 and FF3 components ...

Since - "We can also check metrics presence and don't load our copy unless it's necessary" - then if you are not loading your copy, then where is the conflict?

Actually, how do you *not* load your components unless you have an XPI without them?

If reverting back to the old interface iid's will *really* solve the problem then perhaps that is what we should do ...

If Google folks are using the metrics implementation verbatim, maybe it might be easier to just roll their UI code in one XPI and then as a dependency, install the vanilla metrics xpi all as part of the same one click install. This way they are using the correct metrics version for the target distribution of Firefox ...



Comment 11

10 years ago
Sorry, Pete, I didn't realize that you haven't seen the thread of Alex Polvi and Igor Bazarny. In brief they agreed in the following.

1. To change interfaces names.
2. To make possible compile-time configuration of the metrics -- add "macros for contract id prefixes, prefs prefixes and move class ids to the separate header file, so we can replace everything in our build. File names (config, data), and about module name are also turned into macros, so we get independent clone."

Why 1. Igor wrote:
"our issue is changed interface ids for the same interface names. For some time we plan to distribute extension compatible with Firefox 2 and 3 (same .xpi bundle) so that our users won't need to update on Firefox upgrade. For this we need .xpt files for both metrics extensions to be loaded. We were unable to find a way to hide one file or another (for service implementations we have proxy loader registering version-specific library). We get messages complaining about same interface name registered with different iids and one of versions is unable to find interfaces."

Alex suggested to change interfaces names. I agree with him.
The reverting back the old interface id's would solve the problem if we could freeze the interfaces. On the other hand changing of names of interfaces shouldn't be too hard and will help to avoid the conflict if interfaces are modified.

Why 2.
At present Spectator and Toolbar will share preferences and conractID's if they are installed together. There is no good in any case and Firefox behavior is unpredictable.
(Assignee)

Updated

10 years ago
Attachment #332544 - Flags: review+

Updated

10 years ago
Attachment #332544 - Flags: approval1.9.0.2?
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

I'm pretty sure the metrics extensions is NPOTB and doesn't need approval to land. Can someone confirm?
Attachment #332544 - Flags: review?(pete)
Whiteboard: [npotb?]
(Assignee)

Comment 13

10 years ago
> I'm pretty sure the metrics extensions is NPOTB and doesn't need approval to
> land. Can someone confirm?


That's right, it just needs a review ...

Comment 14

10 years ago
Comment on attachment 332544 [details] [diff] [review]
Rename public interfaces.

Since approval is not needed the request for it is removed.
Attachment #332544 - Flags: approval1.9.0.2?

Comment 15

10 years ago
Created attachment 333417 [details] [diff] [review]
Collect all the public defs into public/nsMetricsModule.h

This patch doesn't interfere with the previous one.

Collect all cntractID's, classID's and preferences that could cause conflict between Spectator and Toolbar into one place -- public/nsMetricsModule.h
Attachment #333417 - Flags: review?(pete)

Updated

8 years ago
Component: Data Collection/Metrics → Data Collection/Metrics
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.