961045 - Modify AOL Repack Configuration

Status: RESOLVED FIXED

(Release Engineering :: Release Requests, defect, P1)

Description

[brian ellis]

Attachment: repack_config_changes_for_AOL.diff

Export file containing patch for AOL, AOL DE and AOL UK partner repacks for Firefox 27

Comment 1

[Mike Connor [:mconnor]]

Hi Brian, can you provide a brief summary of the changes in this diff?

Comment 2

[brian ellis]

Hi - I updated the max version for the toolbars and also there was a new widget added - amazon.

Comment 3

[Mike Connor [:mconnor]]

Brian, the attached diff doesn't include the changes to binary files (and it looks like multiple widgets changed).  Can you export the full changeset so we can pick up the full set of changes?

https://wiki.mozilla.org/Partnering/Repacks/Submitting_Changes#Creating_a_patch should provide details.

Comment 4

[brian ellis]

Mike, the file I attached is what I got when I followed the instructions on the wiki page that you sent.  I added the new files, committed the changes and then did the export just like was listed in the instructions.  Is there a different command to export the full changeset as opposed to the one that I created?  I used hg export tip > repack_config_changes_for_AOL.diff (I also used the revision number in place of 'tip' and got the same diff file).

Comment 5

[Mike Connor [:mconnor]]

Can you retry with "hg export -g tip" to generate the export?

Comment 6

[brian ellis]

Attachment: repack_full_changes_for_AOL.diff

Mike, this is definitely larger than the other file. Hopefully, this is what you are looking for.

Comment 7

[Mike Connor [:mconnor]]

That looks much better.  The giant blobs of text like this are what's needed to give us the new binary files:

? diff --git a/partners/aol/distribution/extensions/{7affbfae-c4e2-4915-8c0f-00fa3ec610a1}/chrome/aoltoolbar.jar b/partners/aol/distribution/extensions/{7affbfae-c4e2-4915-8c0f-00fa3ec610a1}/chrome/aoltoolbar.jar
> index 065e2187f0acccdb47cf7bfabf435cf644487ee3..53c2ddd4e45e157b5b4adffc9f223aebfc323f71
> GIT binary patch
> literal 364141
> zc$|cLV{k4^lXjdF+qP}nwr!l)wr$(CbCR4ScWm3XoqW&kS6jR9)^2TA&7Y~6uBo2B
> ... etc

I'll note that a bunch of the widget zip files changed, expected/intentional?

Comment 8

[brian ellis]

Mike - yes those changes are expected.  Thanks!

Comment 9

[Chris Cooper [:coop] (he/him)]

Comment on attachment 8363782 [details] [diff] [review]
repack_full_changes_for_AOL.diff

Review of attachment 8363782 [details] [diff] [review]:
-----------------------------------------------------------------

Lots of whitespace cleanup, which while painful now will make future reviews easier.

I'm flagging mconnor for follow-up on two js code blocks prior to landing. The same code is present in each of the localized versions.

I also unpacked chrome/aoltoolbar.jar to check the changes. More whitespace cleanup, and adding a custom history button to the toolbar. 

There's also this snippet that I'm unsure about in content/aoltb.js:

- if (type & 2 && this.getPrefBool("homepageprotection.enabled")) {        
+ if (type & 2 && this.getPrefBool("homepageprotection.enabled") && flag == 0)  {        
      // check for home page
           
      if (!this.isDefaultHomepage()) {
          if (!this.getPrefBool("default.homepage.protection") ) {
-             flag += 2;
+             if(this.getPrefBool("resetprompt.skip")){
+                 this.setPrefBool("resetprompt.skip" , false);            
+                 this.setPrefBool("resetprompt.delay" , true);                                                  
+             }else{
+                 flag += 2;
+             }
              //debugMsg("checkDefaults with homepage protecton off   flag+2");
          }else { // if homepage protection is on, immediately reset
              this.onAOLFirstTimeAccept(1, 0, 0, null, null);
              //debugMsg("checkDefaults with homepage protecton on");
          }

I *think* that's honouring homepageprotection.enabled properly, but I could be wrong.

::: partners/aol/distribution/extensions/{7affbfae-c4e2-4915-8c0f-00fa3ec610a1}/components/aolAddonObserver.js
@@ +564,2 @@
>  			    uninstallUrl = strings.GetStringFromName("uninstall.url.default");
> +				var type = strings.GetStringFromName("uninstall.type");	

Flagging this block for further review by mconnor. 

It looks like they're doing the right thing here by grabbing the current search engine for rollback, but want to be sure.

@@ +605,5 @@
>      	    var tempprefs = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
>  	        
>              var tb_searchlabel = strings.GetStringFromName("searchengine.label");
> +			
> +			var tb_newtab, tb_homepage;

Flagging this block for further review by mconnor.

I don't know what are expectations are for setting the homepage and whether they are being met here.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 8363782 [details] [diff] [review]
repack_full_changes_for_AOL.diff

Review of attachment 8363782 [details] [diff] [review]:
-----------------------------------------------------------------

The snippet about homepage protection is correct, based on my reading of the overall context.  This is an AOL feature, not a Firefox feature, but it's a pretty good one, IMO.  I'll get this landed shortly.

::: partners/aol/distribution/extensions/{7affbfae-c4e2-4915-8c0f-00fa3ec610a1}/components/aolAddonObserver.js
@@ +564,2 @@
>  			    uninstallUrl = strings.GetStringFromName("uninstall.url.default");
> +				var type = strings.GetStringFromName("uninstall.type");	

Looks correct to me.  Glad to see this change, actually!

@@ +605,5 @@
>      	    var tempprefs = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
>  	        
>              var tb_searchlabel = strings.GetStringFromName("searchengine.label");
> +			
> +			var tb_newtab, tb_homepage;

This is a relatively small tweak, and makes sure the pref was actually set, rather than defaulted.  It's good from my POV.

Comment 11

[Mike Connor [:mconnor]]

https://hg.mozilla.org/build/partner-repacks/rev/ee23bd108c1a