492392 - Website: on design detail page, set design as default for new Personas user

Status: VERIFIED FIXED

(Websites Graveyard :: getpersonas.com, defect, P2)

Description

[Suneel]

Could we set the design on a detail page as the default for a new Personas user who clicks the download button on that page?

Steps to Reproduce:

1. uninstall personas if it is installed
2. go to a design detail page (eg, http://www.getpersonas.com/persona/815)
3. click the call to action button and install

Desirable Behavior: the design on the page with the call-to-action is set as default 

Actual Behavior: "groovy blue" (same default as homepage download button) is the default. Feedback from the community is that when this happens, many users either 1) don't feel that the download worked, or 2) are frustrated by the experience of researching for the persona design they intended to wear.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

We can do this, but it'll require server-side coding to create custom versions of the extension whose "initial persona" preference is set to the specified persona (i.e. the server will have to unzip the extension, modify its defaults/preferences/personas.js file, zip it back up, and make the modified version available to the user).

And if we serve these modified versions of the extension from getpersonas.com, then users will go through a scarier, more complicated process to install it, since Firefox will tell them that it blocked the site from offering an extension for installation, and they'll have to override that block to install it.

At least unless there's some way to get AMO to serve these modified versions.  I think I recall Chris describing some facility for bouncing extension installation requests through AMO that might be repurposable for this feature.  cc:ing Chris for his thoughts.

Comment 2

[Ryan Doherty (:rdoherty)]

Any chance we could just set a cookie with the persona id and the Personas add-on could read it when installed?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #2)
> Any chance we could just set a cookie with the persona id and the Personas
> add-on could read it when installed?

Hmm, yes, good idea!  The extension could examine cookies on firstrun and set the persona to the one specified by the cookie.  That wouldn't require any custom bundling at all, just some work on the client side, so it would be much easier to implement.

Comment 4

[Toby Elliott [:telliott]]

Would we set the cookie every time the user visits a detail page, or would we be inserting a redirect into the download link? I don't see a cleaner way than those two to get a cookie in there.

I'd argue for the redirect. Without it, we get into some wacky situations where the user is on personas.services and the extension wants to use getpersonas.com

Comment 5

[Ryan Doherty (:rdoherty)]

(In reply to comment #4)
> Would we set the cookie every time the user visits a detail page, or would we
> be inserting a redirect into the download link? I don't see a cleaner way than
> those two to get a cookie in there.

We'd only set the cookie when the user clicks on the button 'get personas now!'.

> 
> I'd argue for the redirect. Without it, we get into some wacky situations where
> the user is on personas.services and the extension wants to use getpersonas.com

Could we have the add-on check for cookies for each domain? Since we don't have any server-side scripting available on getpersonas.com, I don't see how we could do a redirect, set a cookie and install the add-on. I think it would be simplest to check for the cookie on either domain.

Comment 6

[Toby Elliott [:telliott]]

Yeah, JS cookie is probably best, since we can't put code on the server. However, not sure how it'll be able to read multiple domains worth of cookie (unless there's a security hole I'm unaware of)

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> Yeah, JS cookie is probably best, since we can't put code on the server.
> However, not sure how it'll be able to read multiple domains worth of cookie
> (unless there's a security hole I'm unaware of)

The extension has chrome privileges, so it can read the cookies of any domain.

From looking at the various interfaces, it looks like we'd need to use nsICookieManager::enumerator <http://mxr.mozilla.org/mozilla-central/source/netwerk/cookie/public/nsICookieManager.idl> to enumerate all cookies to pull out the relevant one, although nsICookieService::getCookieString <http://mxr.mozilla.org/mozilla-central/source/netwerk/cookie/public/nsICookieService.idl> might also work.

Comment 8

[Suneel]

(In reply to comment #5)
> (In reply to comment #4)
> > Would we set the cookie every time the user visits a detail page, or would we
> > be inserting a redirect into the download link? I don't see a cleaner way than
> > those two to get a cookie in there.
> 
> We'd only set the cookie when the user clicks on the button 'get personas
> now!'.
> '
just to clarify, we would want to set the cookie in two use cases (non-Fx user, and non-Personas Fx user). 


> > 
> > I'd argue for the redirect. Without it, we get into some wacky situations where
> > the user is on personas.services and the extension wants to use getpersonas.com
> 
> Could we have the add-on check for cookies for each domain? Since we don't have
> any server-side scripting available on getpersonas.com, I don't see how we
> could do a redirect, set a cookie and install the add-on. I think it would be
> simplest to check for the cookie on either domain.

Comment 9

[Ryan Doherty (:rdoherty)]

Rough specs:

When the user clicks on the 'get personas now' button it will set a cookie named 'initial_persona' which will contain the same JSON object that is used on getpersonas.com. Example:

{"id":"815","name":"Common Ground Foundation","accentcolor":"#9a0000","textcolor":"#000000","header":"1\/5\/815\/Commons_Persona_header_FINAL.jpg","footer":"1\/5\/815\/Commons_Persona_footer_FINAL.jpg"}

The cookie will expire in 1 day and will either be set for getpersonas.com OR personas.services.mozilla.com. 

If the cookie is found for both domains, whichever has the later expires date takes priority.

Upon install of the add-on or bundle the add-on will check for the cookie and set the persona to the persona in the cookie. The add-on will then delete the cookie(s). 

Let me know if there are any questions.

Comment 10

[Ryan Doherty (:rdoherty)]

Website code committed, should be up on staging soon. Who should this bug be re-assigned to?

Some host file fudging might be necessary to test cookie-reading as the staging server is http://sm-weave-proxy01.services.mozilla.com/

Comment 11

[Suneel]

Ryan, can you confirm that this has been pushed to staging?

Comment 12

[Ryan Doherty (:rdoherty)]

(In reply to comment #11)
> Ryan, can you confirm that this has been pushed to staging?

Yep, it's there. Code is updated on staging every 10 mins.

Comment 13

[Jose E. Bolanos]

Attachment: Patch for service.js

Modifies service.js to set the persona indicated by the initial_persona cookie during the first run.

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 381321 [details] [diff] [review]
Patch for service.js

Almost there!  This works well, there are just a few minor code issues; see below.


>+  _setFirstRunPersona : function() {
>+    let application =
>+      Cc["@mozilla.org/fuel/application;1"].getService(Ci.fuelIApplication);
>+    let extension = application.extensions.get("[e-mail]");

Use the PERSONAS_EXTENSION_ID constant here instead of the literal string.


>+    if (extension.firstRun) {

Nice use of FUEL to determine first run status!  We should do this when checking for first run in personas.js as well (although that is, of course, a different bug that you don't need to worry about).

Note that existing users will trigger this code when they upgrade to a version of Personas that includes it, since FUEL apparently only starts tracking the first run status for an extension the first time you retrieve it from fuelIApplication::extensions, and Personas doesn't currently do so.

I think that's ok, since those users won't have an initial_persona cookie, so the code won't do anything.


>+          let personaJSON = unescape(cookie.value);

unescape is deprecated; use decodeURIComponent instead here.


>+          personaJSON = personaJSON.replace(/\\\//g, "/");

Is this necessary?  JSON.parse should unescape these automatically when parsing the JSON.


Note: when you attach a followup patch with these issues addressed, set the "review" flag to "?" and put my email address in the Requestee field next to the flag so I get notified that the patch has been attached and needs review from me.

Comment 15

[Jose E. Bolanos]

Attachment: Patch for service.js (2)

Thanks for the review! I've corrected the things you mentioned, and I've also added missing code. In the case where several "initial_persona" cookies are found, the code chooses the one with the highest expiration time. It also deletes them now.

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 381387 [details] [diff] [review]
Patch for service.js (2)

>+      let cookie = null;
>+      let selectedCookie = null;
>+
>+      while (cookieEnu.hasMoreElements()) {
>+        cookie = cookieEnu.getNext().QueryInterface(Ci.nsICookie);

This looks and works great!  r=myk

Just one nit: now that "cookie" isn't used outside the while block, declare it inside the block, i.e.:

      while (cookieEnu.hasMoreElements()) {
        let cookie = cookieEnu.getNext().QueryInterface(Ci.nsICookie);

Comment 17

[Jose E. Bolanos]

Attachment: Patch for service.js (3)

Thanks! I did the final correction, as indicated.

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 381423 [details] [diff] [review]
Patch for service.js (3)

Thanks for the patch! r=myk

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Fixed by changeset http://hg.mozilla.org/labs/personas/rev/9d858533de49.

Comment 20

[Carsten Book [:Tomcat]]

verified fixed

Comment 21

[Suneel]

please take a look at this screencast from times 7:00 - 9:30. user installed personas from "Livestrong" detail page, but ended up with groovy blue design. expected behavior was for Livestrong persona to appear.


http://www.usertesting.com/popups/ViewMovie.aspx?file=DpTDN7VtSxk%3d

Comment 22

[Suneel]

assigning to ryan as we suspect that the "Firefox + Personas" button is not setting the cookie appropriately, and therefore not setting the selected persona on first run.

in addition, we have tested "get personas" and "wear this", which both of those work fine.

Comment 23

[Ryan Doherty (:rdoherty)]

Attachment: screenshot of cookie

Checked this in Safari when downloading the bundle, I can see the cookie being set to the correct value. I don't have an IE6 vm to test on right now, but I'm pretty sure it's working there too.

My only guess is the user navigated to getpersonas.com and not www.getpersonas.com, which the add-on might not check for. But I can't see the url bar in the screencast to verify.

Comment 24

[Suneel]

Myk tested this again today and had the same issue, though he might be able to give more detail on steps to recreate.

Comment 25

[Myk Melez [:myk] [@mykmelez]]

These are the steps I use to reproduce on Windows XP:

1. uninstall Firefox, including "personal data and customizations" (i.e. your profile);
2. open IE and go to http://getpersonas.com/gallery/Designer/harry_potter;
3. press the "get personas with firefox" button under the first persona (Harry Potter and the Half-Blood Prince 1);
4. go through the Firefox download, install, and launch process, leaving settings at their default values in the install and launch wizards.

Expected Results: Firefox starts and displays the Harry Potter persona.
Actual Results: Firefox starts and displays the Groovy Blue persona.

Note: I tried on both getpersonas.com and www.getpersonas.com, and I observed the same faulty behavior at both domains.

I wonder if it has anything to do with the website setting the cookie in IE.  Firefox supposedly imports cookies from IE when you first start it, but maybe the cookie doesn't get imported for some reason?

Comment 26

[Ryan Doherty (:rdoherty)]

This looks like a client issue to me. I downloaded the Firefox bundle and installed. If I have Opera AND IE installed, I get a prompt asking me where to import my bookmarks, settings, etc. If only IE installed, I get no prompt.

When I'm prompted to import AND choose IE, it imports all the correct cookies (screenshot to be attached). But it doesn't load the correct persona. 

The cookie is being set and it's the correct data. This is most likely a client or add-on issue.

Comment 27

[Ryan Doherty (:rdoherty)]

Attachment: Firefox imported cookies

Comment 28

[Ryan Doherty (:rdoherty)]

Attachment: Cookie in IE

Comment 29

[Myk Melez [:myk] [@mykmelez]]

I took another look at Suneel's request and can confirm that the cookie shows up on the client.  Here are the steps I took to test on Windows Vista:

1. uninstall Firefox, including "personal data and customizations" (i.e. your
profile);
2. open IE and go to http://getpersonas.com/gallery/Designer/harry_potter;
3. press the "get personas with firefox" button under the first persona (Harry
Potter and the Half-Blood Prince 1);
4. when IE prompts you to download/install Firefox, cancel the download/install;
5. go to http://getfirefox.com/ and install the regular (non-bundled) Firefox, leaving settings at their default values in the install and launch wizards (I followed the Other Systems & Languages > Firefox 3.0.11 links to install Firefox 3.0.11, which is the version of Firefox in the bundle, but presumably this works the same on Firefox 3.5);
6. go to http://getpersonas.com/ and install Personas using the green button on the home page, restarting Firefox when prompted to do so.

After step 5, if you go to Tools > Options > Privacy > Show Cookies..., you can see the cookie in the list, as Firefox has imported it from IE.  Then, when you install Personas and restart Firefox, Personas should see the cookie and apply it, but it doesn't, nor does it delete the cookie, which stays around after step 6.  So this does appear to be a client-side problem.

Comment 30

[Jose E. Bolanos]

I tried reproducing the bug as described by myk numerous times, with no success. 

After I clicked on the "get personas with firefox" for one of the Harry Potter personas, I confirmed that the "initial_persona" cookie was set in IE correctly. Then, after I installed Firefox 3.0.11 (non-bundled) and imported everything from IE, the cookie did not appear under the Tools > Options > Privacy > Show Cookies window. Browsing history was imported correctly, by the way.

I tried with both www.getpersonas.com and getpersonas.com domains.

I also tried with Firefox 3.5, just in case, and could not reproduce it either. Tested in Windows XP.

Any ideas?

Comment 31

[Suneel]

Stephen, we noticed an issue with this fix (see screencast in comment 21) and have attempted to reproduce since with mixed results (see the follow-up comments). Wondering if you might be able to help us identify the issue here, if any.

One recent thought from Myk was that "it's possible there are two separate bugs causing the problem: one that causes Firefox to sometimes not import the cookie, and one that causes Personas to sometimes not use the cookie to set the initial persona."

Comment 32

[Tim Riley [:timr]]

(In reply to comment #30)
> I tried reproducing the bug as described by myk numerous times, with no
> success. 
> 

Jose-  When you do a full removal of Firefox from XP and then a getpresonas (harrypotter) with IE7/XP, you get firefox with the Harry Potter persona? It looks like several of us have tried this and it seems to fail every time.  

I want to be clear here as Myk's steps from comment 29 are more complicated.  As I understand it you can't reproduce the problem at all.  Here is the steps I used.  Sorry if this is too basic, but we need to troubleshoot what is different about your steps as compared to others'.
1) Bring up a XP
  - I used a VMWare VM.  We can probably get you this VM for a one time test
2) Remove any existing Firefox:  
  - Use Windows' Add/Remove Programs 
  - Remove profile. See: http://support.mozilla.com/en-US/kb/Profiles
3) Fire up IE 7
4) Get the Harry Potter persona: http://www.getpersonas.com/gallery/Designer/harry_potter
5) Click on "get personas with firefox"
6) Download to a file
7) Run the file
8) See Firefox with the default persona

If you are getting the Harry Potter persona, can you do these steps on a co-worker's computer or a friend's computer?  It seems easily reproducible here.

Comment 33

[Jose E. Bolanos]

Hi,

We were able to find the problem. The Personas add-on is not able to find the initial_persona cookie because, after being imported from IE, the cookie host gets set to .getpersonas.com (notice the starting dot). To add-on checks that the cookie belongs to one of the authorized hosts, none of them having an initial dot.

I will upload a patch correcting this problem, which ignores initial dots in cookie hosts during the comparison against authorized hosts. It works on our side. Please test it and confirm that it works please when bundled with Firefox.

Thank you

Comment 34

[Jose E. Bolanos]

Attachment: Patch for service.js, cookie host issue

This patch changes the _setFirstRunPersona method of the service module. It ignores the initial dot (.) that may be present in cookie hosts when compared against the authorized Persona hosts. This fixes a bug where, after cookies are imported from IE, the "initial_persona" cookie has the host ".getpersonas.com", and does not match any of the authorized hosts.

Comment 35

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 390602 [details] [diff] [review]
Patch for service.js, cookie host issue

Good catch! r=myk

Comment 36

[Myk Melez [:myk] [@mykmelez]]

I committed this change to the trunk and the client-1.2 branch:

trunk:      http://hg.mozilla.org/labs/personas/rev/a84fe86d5d43
client-1.2: http://hg.mozilla.org/labs/personas/rev/f3cc8fbd3763

I also built both development and 1.2.2rc1 builds:

dev:      https://people.mozilla.com/~cbeard/personas/dist/personas-dev.xpi
1.2.2rc1: https://people.mozilla.com/~cbeard/personas/dist/personas-v1.2.2rc1.xpi

Give the builds a try, and let me know if you notice any problems with them.  This is essentially the only change since Personas 1.2.1.  The only other changes are an update to the Personas version string (1.2.1 -> 1.2.2) and an update to the Thunderbird compatibility max version (3.0b3pre -> 3.0b4pre).

Also, once this is verified, any further problems with application of the initial persona should be filed as new bugs.

Comment 37

[Carsten Book [:Tomcat]]

verified fixed using the steps to reproduce from comment #29. Works as expected and i will also check when we have bundled builds with 1.2.2