Closed Bug 1059097 Opened 10 years ago Closed 10 years ago

Partner repack changes for Yandex for Fx 32 release

Categories

(Release Engineering :: Release Requests, defect)

Product:
Release Engineering
Component:
Release Requests
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: extension-dev, Assigned: mixedpuppy)

Details

(Whiteboard: [partners]) 

All diffs
https://yadi.sk/d/-cTfyEGmaVedQ
Assignee: nobody → mconnor
Our change log looks like following:
- added PowerFM widget for Turkey
- added some inside adverts for widgets
- visual redesign
- removed some widgets
Assignee: mconnor → mixedpuppy
Hi,
I'm currently working on reviewing the updates you've provided, and noticed that there are larger differences between the TR distributions and the others.  Is that difference intentional?
Regards,
Shane
Flags: needinfo?(extension-dev)
Yas, that is expected as only the TR extensions include a widget for PowerFM station - it is not relevant for other countries.
Flags: needinfo?(extension-dev)
I'll have a number of questions while reviewing this, but simply running the addons gives me the following errors that should be addressed.  I'm testing with the latest Fx33 Beta and the RU distribution.

JavaScript warning: xb://f2c0f6e9-297a-6146-aec1-a647aaf35705/native/fx/modules/yauth.jsm, line 274: mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create

System JS : ERROR resource://yandex-vb-mod/Foundation.jsm -> file:///Users/shanec/Desktop/Firefox.app/Contents/MacOS/browser/extensions/vb@yandex.ru/modules/foundation/netutils.js:8 - NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object

System JS : ERROR resource://yandex-vb-app/parts/package.js:49 - NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object

System JS : ERROR resource://yandex-vb-mod/Foundation.jsm -> file:///Users/shanec/Desktop/Firefox.app/Contents/MacOS/browser/extensions/vb@yandex.ru/modules/foundation/xmlutils.js:106 - NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object

this happens a lot:

System JS : ERROR resource://yandex-vb-mod/Foundation.jsm -> file:///Users/shanec/Desktop/Firefox.app/Contents/MacOS/browser/extensions/vb@yandex.ru/modules/foundation/fileutils.js:92 - NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object
Flags: needinfo?(extension-dev)
I also see an http ping request when I click on any of the new toolbar buttons.  This doesn't seem to be user opt-in (or even opt-out).

I've also noticed code that collects a bunch of stats, though I'm not sure what triggers it's execution.  Can you explain the following, outline what data is being sent, and how the user opts-in or out of the data collection?


- there is a lot of new statistics gathering that includes usage:
  - number of saved logins from login manager,
  - daily session time,
  - click counts,
  - history sidebar open,
  - toolbars that are open,
  - boolean flag for whether homepage/search/etc is yandex,
  - more stuff along that line

- click and view stats (I think) on advertising

- tab id ping back.  I don't understand this piece entirely and what exactly it is gathering, but it is sending some kind of tab/browser data back on TabClose events.  A GUID is generated as a tab id, which might be all that gets sent, but it's a bit unclear.

- some other click stats are sent, *presumably* clicks in the widgets (cause of the http requests I see when clicking on toolbar buttons?)
The only issue I see here is a pingback when users click on toolbarbuttons or other items in your pref panel.  Those should be put under the "Help us improve Yandex" preference you have so users can opt-in to giving you that data.  Otherwise, the other data items I mentioned above are all under opt-in or opt-out as far as I can tell, so those are fine.
Thank you for pointing out the problem in our statistics gathering system. We agree that this kind of statistics must be gathered only if user agrees with the option "Help us improve Yandex". We will fix it asap in upcoming release of Elements 8.7 planned due to October. Currently we suggest to launch existing version of Elements 8.5 to avoid possible problems that could be brought by hotfix. After we launch Elements 8.7 all users will be autoupdated and problem with statistics will be solved for whole audience.
Also, about js errors:

JavaScript warning is just a warning, we're working on fixing those and just haven't come to these ones yet. We certainly will in the following versions as right now they do not affect any functions or KPIs of the extension.

The second error is also known and being worked at. It will be fixed in 8.7. It doesn't seem to affect the extension right now so there is no hurry.
Flags: needinfo?(extension-dev)
(In reply to Danil Ivanov from comment #7)
> Thank you for pointing out the problem in our statistics gathering system.
> We agree that this kind of statistics must be gathered only if user agrees
> with the option "Help us improve Yandex". We will fix it asap in upcoming
> release of Elements 8.7 planned due to October. Currently we suggest to
> launch existing version of Elements 8.5 to avoid possible problems that
> could be brought by hotfix. After we launch Elements 8.7 all users will be
> autoupdated and problem with statistics will be solved for whole audience.

I appreciate the desire to release as-is, but I don't think we can do that (will defer to @mconnor on this otherwise).  It should be simple to just check the pref prior to the xhr call.
Flags: needinfo?(mconnor)
(In reply to Danil Ivanov from comment #7)
>  We will fix it asap in upcoming
> release of Elements 8.7 planned due to October.

Is this change going to be as large as 8.5?  If so, it would be really great to start thinking about how to reduce the review into multiple stages somehow, that would help us react faster.
New diffs with a fix
https://yadi.sk/d/kMnR6X67begQw
Sorry, wrong diffs, please don't look at the link in my previous comment
Yeah, sorry again, the diffs are correct.
Here's the link again https://yadi.sk/d/kMnR6X67begQw
(In reply to extension-dev from comment #13)
> Yeah, sorry again, the diffs are correct.
> Here's the link again https://yadi.sk/d/kMnR6X67begQw

I see you used the pref in one location, but I still see xhr requests for toolbar button clicks happening without having opt-in. After further investigation, I see that this is pre-existing and in the current distribution.  Given that, we'll let this drop pass, but we want to see all those clicks dependent on the opt-in for the next drop.  

I'll push this drop into the repo today.
Flags: needinfo?(mconnor) → needinfo?(extension-dev)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Shane, thank you very much, we will put all xhr requests under the checkbox.
Do I get it right that we can start testing a 33.0 candidate, for example, this one?
http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/33.0b7-candidates/
Does it already contain all the changes?
Flags: needinfo?(extension-dev)
(In reply to extension-dev from comment #16)
> Shane, thank you very much, we will put all xhr requests under the checkbox.
> Do I get it right that we can start testing a 33.0 candidate, for example,
> this one?
> http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/33.0b7-candidates/
> Does it already contain all the changes?

Those are standard Firefox builds.

You can find the partner builds here:

https://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/33.0b7-candidates/build1/partner-repacks/

The next build should happen tomorrow or Wednesday.
Hi Shane,
we're looking now at http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/33.0b9-candidates/build1/partner-repacks/yandex-ru/ and it still contains the old extension version
We're tracking this issue in bug 1077374, looks to be a general issue with some build automation changes, we'll get it resolved and new builds ASAP.
Mike, we discovered this 
https://yadi.sk/i/W0bjdRLPcBKHZ
when trying to install Firefox custom build onto Mac OS X 10.9.5
Do you know anything about it?
Component: Custom Release Requests → Release Requests
You need to log in before you can comment on or make changes to this bug.