Closed Bug 935719 Opened 8 years ago Closed 7 years ago
Tabzilla: Create an out of date version warning for users not on the current version
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.
I'm thinking something like the new language bar :kohei made. They're doing a similar thing on SUMO already. Great idea! +1
OS: Windows XP → All
: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.
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
Yeah, let's do this! The Firefox base templates already have the latest and ESR versions so we can use them.
I'm making my Translation Bar code more generic so this warning can be implemented easier. https://github.com/mozilla/bedrock/pull/1258
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?
Assignee: nobody → kohei.yoshino
Severity: normal → enhancement
Status: NEW → ASSIGNED
Sorry for the bugspam -- the Target Milestore was set unexpectedly.
Bug 559111 is relevant. I don't know whether it should be implemented.
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?
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?
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?
Updating the summary since we have removed the -1 logic. (Bug 935673)
Summary: Create an out of date version warning for users not on CurrentVersion -1 → Tabzilla: Create an out of date version warning for users not on the current cersion
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
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.
Summary: Tabzilla: Create an out of date version warning for users not on the current cersion → Tabzilla: Create an out of date version warning for users not on the current version
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.
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?
A messaging for ESR users has been discussed in Bug 952549, but I think those strings will be final.
Curious on that status of this bug?
Bug 952549 has been inactive for a while. We should solve that bug first...
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.
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.
(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.
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.
(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 :)
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
(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.
(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?
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.
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
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.
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?
(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.
So, whats the status of this?
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?
Whiteboard: [kb=1175446] → [kb=1175446] [fxgrowth]
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
Depends on: 988725
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.
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.
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.
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.
:kohei & jpetto: can we come up with a solution soon so we can move forward?
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.
I'll work on this later this week :)
Awesome! I'll be on the lookout for a PR. Thanks :kohei!
: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!
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
Flags: needinfo?(kohei.yoshino) → needinfo?(chrismore.bugzilla)
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.
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?
(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.
Okay, I will add a fallback for 34 and below.
Sorry I'm late... Here's an updated PR for this.
Attachment #8350665 - Attachment is obsolete: true
Thanks for the PRs!
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?
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
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).
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.