I've noticed that nowhere in the migration flow is a "learn more" link for people who are offered a migration. There may be a number of users who want to understand exactly why this migration is being offered and how the new system works, but there's no way for them to learn about it. I was thinking the infobar, or sync prefs, or something, should have a "learn more" link which takes the user to an MDN article. Ryan, do you agree? If not, we can close this as WONTFIX. If so, I guess we need to know exactly where such a link would be added and a new bug for someone to create that content.

A link seems to be the most compatible element given that there are already 2 buttons in some contexts

From bug 1127097, the link is https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/sync-upgrade

* Adds the link in the main infobar and the two prefs infobars in the needs-user state. The link isn't offered if the user already upgraded on a different device because presumably they already have learned more. * Added a link param to Weave's Notification class. * The link is inserted into its xul:notification using the node.ownerDocument.getAnonymousElementByAttribute trick, which is how the other links in xul:notifications work. Ideally there'd be an API... * Links in xul:notifications get the default hyperlink color, which looks terrible at least in OS X notifications with INFO priorities. And not great elsewhere IMO. I took the liberty of changing the color globally to be the same as the notification text, and then I added an underline. It looks better. * I stuck the URL of the support page in a pref. Not sure why these have to be in prefs, but that seems to be the standard. Not sure whether the pref name I chose is OK.

Comment on attachment 8558222 [details] [diff] [review] patch Review of attachment 8558222 [details] [diff] [review]: ----------------------------------------------------------------- r- just for the pref and string, but otherwise looks good. That said though, (a) can we get a screen-shot attached do we can run it past Ryan, and (b) I'm not particularly comfortable with the CSS change - I don't have a problem with it, but my knowledge is such that I don't know what I'd be looking out for. Blair, are you able to sanity check the CSS changes here, or suggest someone else? See comment 4 for the rationale. ::: browser/app/profile/firefox.js @@ +1222,5 @@ > > #ifdef MOZ_SERVICES_SYNC > // The sync engines to use. > pref("services.sync.registerEngines", "Bookmarks,Form,History,Password,Prefs,Tab,Addons"); > +pref("services.sync.fxaMigration.learnMoreURL", "https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/sync-upgrade"); I don't think we want this pref - see later. ::: services/sync/modules/FxaMigrator.jsm @@ +468,5 @@ > }), > > + get learnMoreLink() { > + try { > + var url = Services.prefs.getCharPref(LEARN_MORE_URL_PREF); I think this should just be: Services.urlFormatter.formatURLPref("app.support.baseURL") + "sync-upgrade" (so no need for a new pref - hard-coding the "tail" portion is typical) @@ +474,5 @@ > + return null; > + } > + let sb = Services.strings.createBundle("chrome://weave/locale/services/sync.properties"); > + return { > + text: sb.GetStringFromName("sync.eol.learnMore.label"), I can't see where this string is set in this patch, but can we investigate reusing another string just for uplift (eg, maybe we *do* add a new string for this patch to land in central, but abuse things in a patch for Aurora that reuses?)

FYI here are the three xul:notification priorities on OS X with the patch applied.

Top: Infobar in the main window Middle: In-content prefs Bottom: Dialog prefs

(In reply to Mark Hammond [:markh] from comment #5) > @@ +474,5 @@ > > + return null; > > + } > > + let sb = Services.strings.createBundle("chrome://weave/locale/services/sync.properties"); > > + return { > > + text: sb.GetStringFromName("sync.eol.learnMore.label"), > > I can't see where this string is set in this patch, but can we investigate > reusing another string just for uplift (eg, maybe we *do* add a new string > for this patch to land in central, but abuse things in a patch for Aurora > that reuses?) That's the string created by this changeset: http://hg.mozilla.org/mozilla-central/rev/9e834d0decc2 I really don't think this needs a new string. This existing string is exactly what we want.

(In reply to Drew Willcoxon :adw from comment #8) > That's the string created by this changeset: > http://hg.mozilla.org/mozilla-central/rev/9e834d0decc2 I really don't think > this needs a new string. This existing string is exactly what we want. Ack - sorry about that - I don't know how I managed to screw up my mxr search for it :)

I'm hoping we can use the native prefs style for the native prefs and the in-content style defined by Maslaney for the in-content prefs.

(In reply to Ryan Feeley from comment #10) > I'm hoping we can use the native prefs style for the native prefs and the > in-content style defined by Maslaney for the in-content prefs. You mean for the entire infobar, right? Sadly this bug is just about the link, and we probably need a new bug for the general look of the bar. Or maybe I'm missing what you are saying and just the link in the infobar needs different styling? Ryan, can you please clarify?

Comment on attachment 8558222 [details] [diff] [review] patch Review of attachment 8558222 [details] [diff] [review]: ----------------------------------------------------------------- Given my mistake about the string and the fact that changing the pref is a trivial change I don't need to see again. Assuming rfeeley and unfocused like it, go for it!

Link looks great, but yeah, infobar doesn't match.

Comment on attachment 8558222 [details] [diff] [review] patch Review of attachment 8558222 [details] [diff] [review]: ----------------------------------------------------------------- As discussed on IRC.

Blair, would you mind double checking my CSS?

Comment on attachment 8558840 [details] [diff] [review] patch v2 Review of attachment 8558840 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/themes/linux/global/notification.css @@ +5,5 @@ > @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"); > > +notification, > +.messageText > .text-link { > + color: InfoText !important; Can you double check usage of !important is needed for these? Generally best to avoid them, if possible.

Yeah, the !importants are necessary it seems. I tested on all three platforms. To complement the OS X screenshots I posted earlier, here are before and after screenshots on Windows and Linux, Windows on top, Linux on bottom. The only notification that looks bad with blue links is the red high-priority one, on both platforms. I'll land this patch now, but I can land a follow-up that retains the current blue link color for info- and medium-priority notifications on Windows and Linux, using the main text color only for high-priority notifications, if anyone thinks that's a good idea.

Er, I forgot to remove the text-decoration: underline in the before shots, so imagine the underlines not being there. :-/

Comment on attachment 8559401 [details] [diff] [review] landed unbitrotted patch This patch must land after bug 1097406 lands on Aurora. Approval Request Comment [Feature/regressing bug #]: Not a regression, but part of legacy-Sync-to-FxA migration. [User impact if declined]: This patch adds a "Learn more" link to messsages asking the user to upgrade from old Sync to FxA. Without that link, the user still sees the messages but doesn't have an easy way to understand what they mean. [Describe test coverage new/current, TreeHerder]: Manual testing only. [Risks and why]: Low risk, simply adding a link to existing messages in an infobar and messages in the Sync preferences pane. [String/UUID change made/needed]: None. (The appropriate "Learn more" string already exists in our string files.)

Verified that the 'Learn more' link is present in the migration UI and redirects to https://support.mozilla.org/en-US/kb/firefox-sync-upgrade-frequently-asked-questions?as=u&utm_source=inproduct. I tested on Windows 7 64-bit, Mac OS X 10.10 and Ubuntu 14.04 32-bit using Firefox 37beta4 and latest Developer Edition.