Partner repack changes for Yandex for Fx 32 release

RESOLVED FIXED

Status

Release Engineering
Releases: Custom Builds
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: extension-dev, Assigned: mixedpuppy)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [partners])

(Reporter)

Description

3 years ago
All diffs
https://yadi.sk/d/-cTfyEGmaVedQ
(Reporter)

Updated

3 years ago
Assignee: nobody → mconnor
(Reporter)

Comment 1

3 years ago
Our change log looks like following:
- added PowerFM widget for Turkey
- added some inside adverts for widgets
- visual redesign
- removed some widgets

Updated

3 years ago
Assignee: mconnor → mixedpuppy
(Assignee)

Comment 2

3 years ago
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)
(Reporter)

Comment 3

3 years ago
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)
(Assignee)

Comment 4

3 years ago
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)
(Assignee)

Comment 5

3 years ago
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?)
(Assignee)

Comment 6

3 years ago
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.

Comment 7

3 years ago
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.

Comment 8

3 years ago
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.
(Reporter)

Updated

3 years ago
Flags: needinfo?(extension-dev)
(Assignee)

Comment 9

3 years ago
(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)
(Assignee)

Comment 10

3 years ago
(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.
(Reporter)

Comment 11

3 years ago
New diffs with a fix
https://yadi.sk/d/kMnR6X67begQw
(Reporter)

Comment 12

3 years ago
Sorry, wrong diffs, please don't look at the link in my previous comment
(Reporter)

Comment 13

3 years ago
Yeah, sorry again, the diffs are correct.
Here's the link again https://yadi.sk/d/kMnR6X67begQw
(Assignee)

Comment 14

3 years ago
(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)
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/build/partner-repacks/rev/ea3576df879e
https://hg.mozilla.org/build/partner-repacks/rev/74cb7650782f
https://hg.mozilla.org/build/partner-repacks/rev/1716d70ac140
https://hg.mozilla.org/build/partner-repacks/rev/46f261710b73
https://hg.mozilla.org/build/partner-repacks/rev/5cff704db1e3
https://hg.mozilla.org/build/partner-repacks/rev/26ed47c46f22
https://hg.mozilla.org/build/partner-repacks/rev/5f6cd2a7aae9
https://hg.mozilla.org/build/partner-repacks/rev/6f85653bb763
https://hg.mozilla.org/build/partner-repacks/rev/14c3e667a741
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Comment 16

3 years ago
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)
(Assignee)

Comment 17

3 years ago
(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.
(Reporter)

Comment 18

3 years ago
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.
(Reporter)

Comment 20

3 years ago
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?
You need to log in before you can comment on or make changes to this bug.