935719 - Tabzilla: Create an out of date version warning for users not on the current version

Status: RESOLVED FIXED

(Websites :: Tabzilla, enhancement)

Description

[Tyler Downer [He/Him]]

We should create a warning that notifies users not on CurrentVersion -1 (or current ESR) that they are out of date and should update to the current version immediately.

Comment 1

[Paul [:pmac] McLanahan]

I'm thinking something like the new language bar :kohei made. They're doing a similar thing on SUMO already.

Great idea! +1

Comment 2

[Paul [:pmac] McLanahan]

:kohei and :agibson, what do you think? This doesn't sound that hard to me. We could inject the current Fx version into the tabzilla.js as a variable, we already have good code for getting the Fx version from the UA string, then we do a bar similar to the translation bar :kohei just made. There was a screenshot of SUMO doing nearly this very thing floating around, I'll see if I can find it and attach it here. I just wanted to ask you to see if I'm missing some complexity here. As I see it this should be a pretty easy win :)

Thanks.

Comment 3

[Paul [:pmac] McLanahan]

Tyler sent me the implementation bug from SUMO:

https://bugzilla.mozilla.org/show_bug.cgi?id=651230

And here's the commit that implemented it:

https://github.com/mozilla/kitsune/commit/cdac73c1d6f40dd096573a469fff3bfe12758a4b

Comment 4

[Kohei Yoshino [:kohei]]

Yeah, let's do this! The Firefox base templates already have the latest and ESR versions so we can use them.

Comment 5

[Kohei Yoshino [:kohei]]

I'm making my Translation Bar code more generic so this warning can be implemented easier.
https://github.com/mozilla/bedrock/pull/1258

Comment 6

[Kohei Yoshino [:kohei]]

WIP: https://github.com/kyoshino/bedrock/commit/b9b2356

There will be one information bar at once to avoid confusion, so if this upgrade warning is shown, the translation bar won't appear.

Should we check the minor version also?

Comment 7

[Kohei Yoshino [:kohei]]

Sorry for the bugspam -- the Target Milestore was set unexpectedly.

Comment 8

[Kohei Yoshino [:kohei]]

Bug 559111 is relevant. I don't know whether it should be implemented.

Comment 9

[Alex Gibson [:agibson]]

I think this sounds great! 

+1 for only showing one popup at a time, that was going to be my only recommendation.

I believe we only check the major version in other areas of bedrock, so probably best stuck to the same behavior for consistency and to avoid mixed messaging?

Comment 10

[Kohei Yoshino [:kohei]]

So we can simply check the major version. Here's an updated commit. Embedded the latest versions as variables as :pmac said, in order to make the version check work also on other sites. Still needs some tests.

https://github.com/kyoshino/bedrock/commit/e0e10e2

I was thinking about the design. It should be prominent... with a yellow background?

Comment 11

[Chris More [:cmore]]

What is the reason to make this warning be part of the translation bar and not just put it in Tabzilla itself? If it is going to be tabzilla, why is it blocked by bug 90643?

Comment 12

[Kohei Yoshino [:kohei]]

Updating the summary since we have removed the -1 logic. (Bug 935673)

Comment 13

[Tyler Downer [He/Him]]

That's fine, please make sure to loop me into other update policy related discussions as this currentversion -1 logic was being used in other places and it's important we remain consistent. Thanks

Comment 14

[Kohei Yoshino [:kohei]]

Attachment: Pull Request on GitHub

Comment 15

[Kohei Yoshino [:kohei]]

Opps, a typo in the summary.

(In reply to Tyler Downer [:Tyler] from comment #13)
> That's fine, please make sure to loop me into other update policy related
> discussions as this currentversion -1 logic was being used in other places
> and it's important we remain consistent. Thanks

According to pmac's comment 1, the -1 logic has been used also on SUMO. I'll file a bug to remove it.

Comment 16

[Kohei Yoshino [:kohei]]

Looks like the version check logic on SUMO is maintained manually. It could be replaced with this Update Bar for consistency and maintainability. Added Bug 922742 Comment 20.

Comment 17

[Pascal Chevrel:pascalc (PTO until April 26)]

Looking at the patch, this uses 3 strings, 2 of them we already have in the firefox/new age so we could just import them in tabzilla.lang, the last one we don't have, it is:

Update later.

That means that we need to get that translation done for all of our Firefox locales soon so as to cover as much as our userbase as possible. Are the strings used for this new feature complete and final?

Comment 18

[Kohei Yoshino [:kohei]]

A messaging for ESR users has been discussed in Bug 952549, but I think those strings will be final.

Comment 19

[Tyler Downer [He/Him]]

Curious on that status of this bug?

Comment 20

[Kohei Yoshino [:kohei]]

Bug 952549 has been inactive for a while. We should solve that bug first...

Comment 21

[Alex Gibson [:agibson]]

Given what's now being proposed in bug 988725, is Tabzilla still a suitable place to house this functionality? We need to consider that Tabzilla can be used on any website, and not just *.mozilla.org domains. 

Just a thought for consideration.

Comment 22

[Tyler Downer [He/Him]]

Alex,
That is an in-product change, so for the millions of users on out of date versions that we are trying to reach with this fix, it won't help. Also I see no problem with making tabzilla give a warning and another site using the same warning, the more warnings the better.

Comment 23

[Alex Gibson [:agibson]]

(In reply to Tyler Downer [:Tyler] from comment #22)
> Alex,
> That is an in-product change, so for the millions of users on out of date
> versions that we are trying to reach with this fix, it won't help. Also I
> see no problem with making tabzilla give a warning and another site using
> the same warning, the more warnings the better.

What I'm trying to say is that if this in-product change is restricted to a white-list of *.mozilla.org domains only, it would give inconsistent results for ESR users when Tabzilla is used on any other type of domain. There might not be many of those sites, but it's something to consider.

Comment 24

[Tyler Downer [He/Him]]

Yes, I hear you, but we have to keep in mind that ESR users are 1. very few and 2, not our main objective of support. They have their own IT department, and they should be managing updates, so the ESR experience is the least of my concerns when compared to all the regular users.

Comment 25

[Alex Gibson [:agibson]]

(In reply to Tyler Downer [:Tyler] from comment #24)
> Yes, I hear you, but we have to keep in mind that ESR users are 1. very few
> and 2, not our main objective of support. They have their own IT department,
> and they should be managing updates, so the ESR experience is the least of
> my concerns when compared to all the regular users.

Roger, I just thought I'd point it out :)

Comment 26

[Alex Gibson [:agibson]]

Holly, to proceed with l10n for this bug can we please get confirmation on what the messaging should be here? Seems some of this was discussed in Bug 952549 but we're not quite clear on what the final strings should be. Thanks

Comment 27

[Kohei Yoshino [:kohei]]

(In reply to Alex Gibson [:agibson] from comment #21)
> Given what's now being proposed in bug 988725, is Tabzilla still a suitable
> place to house this functionality? We need to consider that Tabzilla can be
> used on any website, and not just *.mozilla.org domains. 

Right, SUMO may want this, but most sites don't. Tabzilla.setupUpdatebar should not be in tabzilla.js. We can just *plug in* the function to Tabzilla's generic Infobar functionality. I'll update my pull request then.

Comment 28

[Alex Gibson [:agibson]]

(In reply to Kohei Yoshino [:kohei] from comment #27)
> Right, SUMO may want this, but most sites don't. Tabzilla.setupUpdatebar
> should not be in tabzilla.js. We can just *plug in* the function to
> Tabzilla's generic Infobar functionality. I'll update my pull request then.

Kohei, should we not wait to see the outcome of bug 988725 as well as discuss this with others first before updating your code?

Comment 29

[Kohei Yoshino [:kohei]]

Oh I just updated my code :)

Bug 988725 won't block this work. Once the bug is solved, we can show ESR users a different message, but until then Firefox 24 users won't get any notifications.

As you see in the updated PR, SUMO can utilize updatebar.js if they want.

Comment 30

[Tyler Downer [He/Him]]

If I understand correctly, this means that each site that wants this (MDN, AMO, SUMO, etc.) will have to specifically enable it? Or will this just be for all mozilla.org sites? Just determining the logistics

Comment 31

[Kohei Yoshino [:kohei]]

Yes, this is an opt-in functionality like the Translation Bar (Bug 906943).
http://bedrock.readthedocs.org/en/latest/tabzilla.html#translation-bar

This Update Bar may not be needed by many sites, so I have excluded the code from tabzilla.js. Instead, if SUMO or other sites want to use it, they have to include updatebar.js then add an HTML attribute.

Comment 32

[Holly Habstritt Gaal [:Habber]]

We've been discussing this a bit further on IRC. Treating 10 & 17 users all as not up-to-date makes sense. However, the Tabzilla banner style notification should be disabled on /new. (There is a separate in-page conditional message already existing on this page)

The banner copy used in bug 952549 was approved by Matej.  (See attachment: https://bug952549.bugzilla.mozilla.org/attachment.cgi?id=8356201)  "Update to stay fast and safe" links to /new and should be a button. "Later" should simply be a text link that closes the notification. 

Does this work for everyone?

Comment 33

[Kohei Yoshino [:kohei]]

(In reply to Holly Habstritt [:Habber] from comment #32)
> the Tabzilla banner style notification should be disabled on /new.

Yes, Tabzilla's Update Bar will be disabled on /firefox/new/. I should have commented it in this bug earlier.

> The banner copy used in bug 952549 was approved by Matej.  (See attachment:
> https://bug952549.bugzilla.mozilla.org/attachment.cgi?id=8356201)  "Update
> to stay fast and safe" links to /new and should be a button. "Later" should
> simply be a text link that closes the notification. 

Looks like the current label in my PR is "Update later." as Habber recommended in Bug 952549 Comment 10. I'll change it to "Later" -- it's a simple, common label and I personally like it!

"Update to stay fast and safe" is the same label as /firefox/new/ so we won't need another l10n.

Comment 34

[Tyler Downer [He/Him]]

So, whats the status of this?

Comment 35

[Tyler Downer [He/Him]]

Any progress?

Comment 36

[Chris More [:cmore]]

I'm going to pull this into Firefox Growth Team as we have found big gains from doing this on key websites. If nearly every website at Mozilla uses tabzilla, this bug could possibly get more older Firefox users to upgrade to the latest version more than one-off website notification.

:jpetto: can you scope out how much work this project would be?

Comment 37

[Kohei Yoshino [:kohei]]

I've found my WIP (linked in my comment 6) here but anyway, I believe this also depends on Bug 988725.
https://github.com/kyoshino/bedrock/commit/27966519eab78e5020cd247415acd0f2e37ae8d9

Comment 38

[Jonathan Petto [:jpetto]]

I'll (re-)investigate dependencies and steps tomorrow and report back.

It'll be a huge win if we can use Kohei's WIP branch.

Comment 39

[Jonathan Petto [:jpetto]]

After reading through bug 988725, it appears we have a few options:

1) Wait for bug 988725 to be resolved so we can properly identify ESR users. Need to inquire about timeline.

2) Implement this update bar now, knowing that ESR users will be incorrectly prompted to update. Seems acceptable as per comment 24, but not ideal.

3) Use the UI Tour JS (https://github.com/Unfocused/mozilla-uitour) to get the full version (as mentioned in comment 46 on bug 988725 - https://bugzilla.mozilla.org/show_bug.cgi?id=988725#c46). Asked for verification that this option is feasible in bug 988725.

If available today, option #3 seems to be the best. The extra JS dependency isn't ideal, but would likely be temporary (assuming bug 988725 gets completed/resolved).

:kohei - Your WIP branch looks pretty close. How much more work do you think is needed to complete? Assuming we move forward with option #2 or #3 above, do you have time to finish? If not, I could take it on.

Comment 40

[Kohei Yoshino [:kohei]]

As I commented in Bug 988725, Mozilla.UITour.getConfiguration("appinfo", callback) can be probably used though it's still available only on Nightly (Firefox 35). We can copy a part of mozilla-uitour.js into tabzilla.js so there would be no extra JS dependency. I'll work on this and Bug 939470 (the same feature on /firefox/new/) next week.

Comment 41

[Chris More [:cmore]]

My concern about the UITour variable is that it would only work on whitelisted domains and tabzilla is across almost all websites. I would prefer to prompt for a Firefox upgrade on Firefox visitors that are older than 3 major versions and ignoring ESR builds.

Comment 42

[Chris More [:cmore]]

:kohei & jpetto: can we come up with a solution soon so we can move forward?

Comment 43

[Jonathan Petto [:jpetto]]

I think our best bet is to do as :cmore said - show the notice to users who are 3 major versions behind or more, while acknowledging that ESR users may see an incorrect message. If and when it becomes feasible to check for ESR users, we can update the code.

:kohei - Do you think you'll have time to finish your WIP branch this week? If not, I can take it on so we can get it out soon.

Comment 44

[Kohei Yoshino [:kohei]]

I'll work on this later this week :)

Comment 45

[Jonathan Petto [:jpetto]]

Awesome! I'll be on the lookout for a PR.

Thanks :kohei!

Comment 46

[Jonathan Petto [:jpetto]]

:kohei - Think you'll have a PR ready for review tomorrow? Don't mean to be a pest, but we do want to get this out as soon as possible. Let me know if you're backed up or stuck on another task.

Thanks!

Comment 47

[Kohei Yoshino [:kohei]]

I'm still thinking about consistency with the version check on /firefox/new/ (Bug 939470).

/firefox/new/ could probably use the UITour API to always show an accurate message, so I think it would be nice to share the same code/experience with this Update Bar, rather than showing the bar only to visitors on older than 3 major versions.

The Update Bar may only be used on www.mozilla.org and support.mozilla.org anyway, which are currently whitelisted and also requested in Bug 988725, so using the API won't be a major issue.

:cmore - what do you think?

I have tested my current WIP on Nightly, confirmed that the latest API provided the appinfo and the Update Bar was working with a test data set:

https://github.com/kyoshino/bedrock/compare/bug-935719-updatebar

Comment 48

[Kohei Yoshino [:kohei]]

Attachment: screenshot

Comment 49

[Jonathan Petto [:jpetto]]

Hm, so your proposed fix would only work on Fx 35 and up, correct? As far as I understand, we need to target all the existing users who are on Fx 29 or lower. There are many users who have not been updating and may not know that they are out of date. Using the UITour API is certainly the better technical solution, but wont help the users who are currently behind.

Comment 50

[Paul [:pmac] McLanahan]

A combination of the two should work as well right? Use the accurate information if available, and fall back to the "3 major versions back" method if not?

Comment 51

[Jonathan Petto [:jpetto]]

(In reply to Paul McLanahan [:pmac] from comment #50)
> A combination of the two should work as well right? Use the accurate
> information if available, and fall back to the "3 major versions back"
> method if not?

This absolutely sounds like the way to go.

Comment 52

[Kohei Yoshino [:kohei]]

Okay, I will add a fallback for 34 and below.

Comment 53

[Kohei Yoshino [:kohei]]

Attachment: Pull Request on GitHub

Sorry I'm late... Here's an updated PR for this.

Comment 54

[Chris More [:cmore]]

Thanks for the PRs!

Comment 55

[Chris More [:cmore]]

Can we see a working demo of this after it is merged and will mozilla.org require an additional PR to opt-in to the notice?

Comment 56

[Jonathan Petto [:jpetto]]

I can probably get the branch on a demo server today - I think two will free up after the push. I'll update this bug when it's up.

The PR enables the update bar by default for all of mozilla.org. Individual pages can opt out if/when necessary. The only page opting out initially will be /firefox/new/ (as it already has the same logic). Other sites using Tabzilla can configure which notification bars they want in their settings:

https://github.com/mozilla/bedrock/pull/2339/files#diff-7194d3de4cc9718f18e49e996f1e50b0R1057

Comment 57

[Jonathan Petto [:jpetto]]

The branch is now up on demo5:

https://www-demo5.allizom.org/en-US/

If you set your user agent string to a Firefox at version 30 or below, you'll see the update bar (unless visiting the /firefox/new/ page).

Comment 58

[[github robot]]

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

https://github.com/mozilla/bedrock/commit/23b39bade21228ac054e3c8f428c2c06f7f23310
Fix Bug 935719 - Tabzilla: Create an out of date version warning for users not on the current version

https://github.com/mozilla/bedrock/commit/6a0f68882f084bb038f2874ec021b7d38351a7dc
Merge pull request #2339 from kyoshino/bug-935719-updatebar

Fix Bug 935719 - Tabzilla: Create an out of date version warning for users not on the current version