1048689 - sync cta doesn't appear on whats new

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Jennifer Bertsch [:jbertsch]]

Attachment: no CTA

use case:

i just updated to the latest version and the “whatsnew” page loaded showing a promo for Firefox Accounts
 
however, it looks like there’s a bug as the button doesn’t display that would kick off the sign up process

this instance of the browser is not currently signed in

Comment 1

[Jennifer Bertsch [:jbertsch]]

Note:  Cmore reports: 

I have new Sync installed and when I go to:

https://www.mozilla.org/en-US/firefox/31.0/whatsnew/?oldversion=30.0

Comment 2

[Jennifer Bertsch [:jbertsch]]

Hi Raymond - Do you have time to look a this one on August 5?  Thx!

Comment 3

[Jennifer Bertsch [:jbertsch]]

Attachment: comment #1 what cmore sees

Comment 4

[Chris More [:cmore]]

Ok, I have diagnosed this somewhat.

The sync-cta div is hidden by default in the CSS:

https://github.com/mozilla/bedrock/blob/master/media/css/firefox/australis/australis-page-common.less#L176

There is a conditional in the JS that checks to see if config.setup is a Boolean and false, then the JS sets sync-cta to visible:

https://github.com/mozilla/bedrock/blob/master/media/js/firefox/australis/tour.js#L24

From reading over the docs on Mozilla.UITour, the Mozilla.UITour.getConfiguration('sync', function (config) should check if the user is using Sync. What this all means is that the sync-cta is *only* visible when config.setup for sync is false, which means the users is not using Sync.

What I can't find is if the getConfiguration can differentiate between old Sync and new Sync. Does anyone have an old Sync setup that can visit the page in comment 1?

I checked GA for the key 06 and all values are either true or false and there are no other values. So, all users should be dropping into one of those two conditions.

Comment 5

[Chris More [:cmore]]

Alex: can you check out comment 4?

Comment 6

[Alex Gibson [:agibson]]

Hi all,

Yes, getConfiguration can't differentiate between new Sync and old sync, that's one limitations.

The current page logic checks to see if Sync is enabled, and then it gets either a "true" or "false" value returned by the browser, which then shows the appropriate page content depending on this value. It's working fine for me too when I test, but I wonder if something happened whereby a value other than "true" or "false" got returned - perhaps an edge case?

To find out what the value is, they can follow these steps:

1.) go to https://www.mozilla.org/en-US/firefox/31.0/whatsnew/?oldversion=30.0
2.) click Tools -> Web Developer -> Web Console
3.) paste the following in the console

Mozilla.UITour.getConfiguration('sync', function (config) { console.dir(config.setup); });

The first line it outputs should be 'undefined', which is expected, but after that the next line should be either 'true' or 'false'.

If there is a condition where config.setup is returning any other value (like undefined for instance), that's a Firefox bug. However we could still put an extra bit of logic in the page that says when in doubt, show the button anyway.

- Alex

Comment 7

[Chris More [:cmore]]

I thought of the non boolean value too, but I'm only seeing true or false in in GA. If there was another value, wouldn't we see it in the GA custom event value?

Comment 8

[Alex Gibson [:agibson]]

That is true yes, so maybe we're looking at an edge case where the callback isn't firing? We've had a similar bug with getConfiguration before. CC'ing Matt who may be able to help diagnose the issue if there's a problem on the Firefox side.

Matt - we may have an issue where getConfiguration for sync is not working as expected. Can you help diagnose? Thanks

Comment 9

[Alex Gibson [:agibson]]

As an interim fix, we could still show the sync cta button by default, and then only hide it if config.setup = true. But this would then be assuming the UITour API is working as expected for that user.

Comment 10

[Matthew N. [:MattN]]

Could I get the following info please:
1) "User set" browser.uitour.* preferences

2) The value of services.sync.username in about:config

3) In the "Web Console", click the gear icon and check "Enable chrome and add-on debugging" under advanced settings. Then Tools > Web Developer > "*Browser* Console" and run the following while the tour page is the selected tab:
  console.log(content.location.toString(), Services.perms.testPermissionFromWindow(content, "uitour"))
It will output the tab's URL and whether it has the correct permission within Firefox to use UITour.

4) Output from comment 6.

5) Is it possible to use the data in GA to see how many *Firefox* users for applicable versions see the page but don't have the value true or false for config.setup? e.g. 20M page loads of Firefox 31 on 
https://www.mozilla.org/en-US/firefox/31.0/whatsnew/?oldversion=30.0 and then look at the config.setup counts and add them to see if it adds up to 20M.

6) Clear the *Browser* Console window enabled in step 3 and then refresh the tour tab to see if any JavaScript errors appear in that window.

7) Was this profile used for setting up A/B testing? I believe that required changing some preferences and may have got the profile in a weird state.

Those are the debugging ideas I have for now.

Comment 11

[Alex Gibson [:agibson]]

Thanks for the quick reply, Matt

(In reply to Matthew N. [:MattN] from comment #10)
> 7) Was this profile used for setting up A/B testing? I believe that required
> changing some preferences and may have got the profile in a weird state.

I believe this was from Chris Beard's profile, as he originally reported the bug.

Comment 12

[Chris More [:cmore]]

Gareth: can you help with #5 in comment 10?

Comment 13

[Gareth Cull [:garethc]]

Sure thing. I'll take a look at this and will update the bug with my findings.

Comment 14

[Gareth Cull [:garethc]]

So, I'm seeing that 91% of sessions to the whatsnew page in the past 30 days are being assigned a value of either true or false. We aren't seeing any other values being assigned within this custom variable, which makes me think visitors are being correctly categorized. I'm not seeing any undefined value appear in the custom variable.

There are 9% of sessions that are not actually getting assigned a custom variable. This may be due to when we trigger the event that stores the custom variable (i believe it was the last thing to load and after the original trackpageview call to GA). So, there may be cases where the custom variable event may not be triggered such as if the user immediately bounces or takes an action before that event is fired.

Comment 15

[Matthew N. [:MattN]]

(In reply to Gareth Cull [:garethc] from comment #14)
> There are 9% of sessions that are not actually getting assigned a custom
> variable. This may be due to when we trigger the event that stores the
> custom variable (i believe it was the last thing to load and after the
> original trackpageview call to GA). So, there may be cases where the custom
> variable event may not be triggered such as if the user immediately bounces
> or takes an action before that event is fired.

Thanks. There has been a bug lingering for years without STR that causes permissions to be lost and my fear is that this bug is caused by that issue so I was curious to see how many users could be affected. It seems like at most 9% (of the users who saw the whatsnew page). I've personally hit the bug multiple times over the past 5 years. #3 in comment 10 will be an indicator if this is related.

Comment 16

[Jennifer Bertsch [:jbertsch]]

(In reply to Matthew N. [:MattN] from comment #15)
> (In reply to Gareth Cull [:garethc] from comment #14)
> > There are 9% of sessions that are not actually getting assigned a custom
> > variable. This may be due to when we trigger the event that stores the
> > custom variable (i believe it was the last thing to load and after the
> > original trackpageview call to GA). So, there may be cases where the custom
> > variable event may not be triggered such as if the user immediately bounces
> > or takes an action before that event is fired.
> 
> Thanks. There has been a bug lingering for years without STR that causes
> permissions to be lost and my fear is that this bug is caused by that issue
> so I was curious to see how many users could be affected. It seems like at
> most 9% (of the users who saw the whatsnew page). I've personally hit the
> bug multiple times over the past 5 years. #3 in comment 10 will be an
> indicator if this is related.

Thanks, MattN and Gareth.

Cmore asked Chris to bring his laptop by so that Cmore can get the information you asked for in Comment 10, Matt N.

Comment 17

[Jennifer Bertsch [:jbertsch]]

Hi Alex-

As a temporary fix - is it possible to force the sync CTA button for the 9% of times when the browser doesn't return a custom variable?

Thx,
Jen

Comment 18

[Chris More [:cmore]]

Alex: would reversing the logic help so that it is displayed by default and you only hide the cta on exception?

Comment 19

[Alex Gibson [:agibson]]

What I would suggest is displaying the CTA button by default, but link it to the firefox/sync page for those users. There's little point in showing the button if it's a profile pref issue like Matt suggests, as clicking the button would not do anything (this is why it currently does not show by default). Linking it to firefox/sync should act like a form of progressive enhancement. So instead if highlighting sync in the menu for those users, it falls back to a regular link where they can still learn about how to sigh up.

Sound ok?

Comment 20

[Jennifer Bertsch [:jbertsch]]

(In reply to Alex Gibson [:agibson] from comment #19)
> What I would suggest is displaying the CTA button by default, but link it to
> the firefox/sync page for those users. There's little point in showing the
> button if it's a profile pref issue like Matt suggests, as clicking the
> button would not do anything (this is why it currently does not show by
> default). Linking it to firefox/sync should act like a form of progressive
> enhancement. So instead if highlighting sync in the menu for those users, it
> falls back to a regular link where they can still learn about how to sigh up.
> 
> Sound ok?

Sounds like a solid plan, Alex.  Let's do that.  Thank you!

Comment 21

[Chris More [:cmore]]

(In reply to Alex Gibson [:agibson] from comment #19)
> What I would suggest is displaying the CTA button by default, but link it to
> the firefox/sync page for those users. There's little point in showing the
> button if it's a profile pref issue like Matt suggests, as clicking the
> button would not do anything (this is why it currently does not show by
> default). Linking it to firefox/sync should act like a form of progressive
> enhancement. So instead if highlighting sync in the menu for those users, it
> falls back to a regular link where they can still learn about how to sigh up.
> 
> Sound ok?

This sounds good. This sounds a bit more "safe" with whatever would cause this bug. We'll have to wait until we can see the value in cbeard's laptop. I am sure he is not the only one.

Comment 22

[Chris Beard]

(In reply to Matthew N. [:MattN] from comment #10)
> Could I get the following info please:
> 1) "User set" browser.uitour.* preferences
> 
all of these preferences are default with the exception of "browser.uitour.whitelist.add.260" which is set to a blank string

> 2) The value of services.sync.username in about:config
preference doesn't exist -- sync is not set up

> 3) In the "Web Console", click the gear icon and check "Enable chrome and
> add-on debugging" under advanced settings. Then Tools > Web Developer >
> "*Browser* Console" and run the following while the tour page is the
> selected tab:
>   console.log(content.location.toString(),
> Services.perms.testPermissionFromWindow(content, "uitour"))
> It will output the tab's URL and whether it has the correct permission
> within Firefox to use UITour.

ReferenceError: Services is not defined

> 
> 4) Output from comment 6.

undefined

(and there's no "true" or "false" after it)



> 6) Clear the *Browser* Console window enabled in step 3 and then refresh the
> tour tab to see if any JavaScript errors appear in that window.

No error, but a warning: Use of Mutation Events is deprecated. Use MutationObserver instead.

> 7) Was this profile used for setting up A/B testing? I believe that required
> changing some preferences and may have got the profile in a weird state.

Not that I'm aware of.  This is a new profile from April.

Comment 23

[Matthew N. [:MattN]]

(In reply to Chris Beard from comment #22)
> (In reply to Matthew N. [:MattN] from comment #10)
> > Could I get the following info please:
> > 1) "User set" browser.uitour.* preferences
> > 
> all of these preferences are default with the exception of
> "browser.uitour.whitelist.add.260" which is set to a blank string

OK, that's expected and means that the permission was added to the permission database.

> > 3) In the "Web Console", click the gear icon and check "Enable chrome and
> > add-on debugging" under advanced settings. Then Tools > Web Developer >
> > "*Browser* Console" and run the following while the tour page is the
> > selected tab:
> >   console.log(content.location.toString(),
> > Services.perms.testPermissionFromWindow(content, "uitour"))
> > It will output the tab's URL and whether it has the correct permission
> > within Firefox to use UITour.
> 
> ReferenceError: Services is not defined

I double-checked with Chris Beard in-person and we ran this command in the Browser Console and the number that was output was "0" which is UNKNOWN_ACTION which means no permission is set. This is what I suspected was the problem and means the permissions was lost at some point.

I'm going to make sure there is a bug on file to get the lost permissions issue looked into. An easy workaround would be to hard-code the whitelist for the tour or use preferences for it instead. We really should fix the permission manager backend so it doesn't lose permissions as that also affects add-on and theme installations among other things.

Comment 24

[Matthew N. [:MattN]]

Hello Chris,

I think I may have found an explanation for this: 

In this profile, have you ever used the "Clear Recent History" dialog (found under the History menu/button) or the option in Privacy preferences to "Clear history when Firefox closes"? If you have used either of those, do you know if you ever had the "Site Preferences" checkbox checked? You can check the most recent usage of those dialogs in about:config by filtering with ".siteSettings" and seeing if either of the two privacy.*.siteSettings preferences are true.

If you have used either of those functions then the core problem is bug 506446.

Comment 25

[Chris Beard]

(In reply to Matthew N. [:MattN] from comment #24)

> In this profile, have you ever used the "Clear Recent History" dialog (found
> under the History menu/button) or the option in Privacy preferences to
> "Clear history when Firefox closes"? 

Yes. And I checked and privacy.cpd.siteSettings is user set to true.

Comment 26

[Matthew N. [:MattN]]

Thanks, I think this mystery is solved then. Fixing bug 506446 (which I've escalated now) is the main issue but bug 771630 also adds to the problem because the time period doesn't affect which "Site Settings" are cleared, they are simply all removed. I'll file another bug to re-add the removed permissions for users were affected by those two bugs.

Comment 27

[Chris Beard]

Thanks!  Is there any chance this could also be the root cause of application updates failing for a % of Firefox users? Does that update service use a similar whitelist? (Sorry, I couldn't find the related bug number.)

Comment 28

[Matthew N. [:MattN]]

(In reply to Chris Beard from comment #27)
> Thanks!  Is there any chance this could also be the root cause of
> application updates failing for a % of Firefox users? Does that update
> service use a similar whitelist? (Sorry, I couldn't find the related bug
> number.)

I don't believe it would be related at this time because the only pre-installed whitelists in the permission manager are for the UITour (e.g. this bug), and add-on/app installation:

/browser/app/profile/firefox.js
    line 247 -- pref("xpinstall.whitelist.add", "addons.mozilla.org");
    line 248 -- pref("xpinstall.whitelist.add.180", "marketplace.firefox.com");
    line 258 -- pref("browser.uitour.whitelist.add.260", "www.mozilla.org,support.mozilla.org");
    line 259 -- pref("browser.uitour.whitelist.add.340", "about:home");

Comment 29

[Alex Gibson [:agibson]]

Attachment: GitHub pull request

Interim fix that links the Sync CTA button on /whatsnew and /firstrun to the `firefox/sync` marketing page, should the UITour API not work.

Comment 30

[[github robot]]

Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/ba88fd60f0a93fa105638555b2468f44016650d5
[bug 1048689] sync cta doesn't appear on whats new

https://github.com/mozilla/bedrock/commit/4a8b1a1dcb7e3289c294e79a243515d559ec0ab2
Merge pull request #2210 from alexgibson/bug-1048689-add-fallback-sync-cta-whatsnew

[bug 1048689] sync cta doesn't appear on whats new

Comment 31

[Kohei Yoshino]

Closing as per Comment 30.