Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change download buttons to point to /firefox/new/ #2196

Closed
wants to merge 2 commits into from

Conversation

sgarrity
Copy link
Contributor

@sgarrity sgarrity commented Aug 3, 2014

Download links will point to the second scene of download page
with the url fragment /firefox/new/#download-fx
Bug 874913

@sgarrity
Copy link
Contributor Author

sgarrity commented Aug 4, 2014

After discussing on the phone: I need to check with l10n that the settings.LOCALES_WITH_TRANSITION list is relevant to the /firefox/new page, and not only the old products/download.html page.

@flodolo
Copy link
Contributor

flodolo commented Aug 4, 2014

It's not completely relevant: LOCALES_WITH_TRANSITION has several locales (bn-BD, bn-IN, si, to name a few) that are not enabled for /firefox/new because the page has never been localized.

On the other hand, is missing locales that we added lately (az, dsb, etc.).

@sgarrity
Copy link
Contributor Author

sgarrity commented Aug 4, 2014

Here's a quick summary of our discussion on vidyo today:

The ye-olde PHP-based buttons used to do client-side locale detection and output a download link for that locale (either direct, or through the old transition page).

The /firefox/new/ page download does not do client-side locale detection except to redirect to /LOCALE/firefox/new if there is no locale in the URL.

There are locales in which Firefox is available for download that the /firefox/new page is NOT available in. See http://l10n.mozilla-community.org/~pascalc/langchecker/?locale=all&website=0&file=firefox/new.lang

As a result, if this PR were to land as is, and a visitor had a locale that was not enabled for /firefox/new, they would have to manually choose the Systems & Languages link.

I wondered if we should "enable" ALL locales for /firefox/new/ and consider the download button strings (at least) as a requirement to ship Firefox in a given locale.

Keeping these code pathes simple, understandable, and testable is key, given how important the download process is.

@jgmize also aptly suggested that we get QA testing this as soon as possible.

@jgmize
Copy link
Contributor

jgmize commented Aug 7, 2014

I submitted #2215 to enable all locales on bedrock for /firefox/new

@jgmize
Copy link
Contributor

jgmize commented Aug 7, 2014

@sgarrity once you have Jenkins passing will you please put this on one of the demo environments?

Download links will point to the second scene of download page
with the url fragment /firefox/new/#download-fx
Bug 874913
@sgarrity
Copy link
Contributor Author

sgarrity commented Aug 7, 2014

@jgmize @pmclanahan @flodolo So, if we enable all locales for /firefox/new/ as per PR #2215, is the LOCALES_WITH_TRANSITION list still relevant? See:

# Locals with transitional download pages

I wondered if it might make sense to send all downloaders, regardless of locale, to /firefox/new/#download-fx - even if they would see an en-US post-download page.

Also, does PR #2215 affect whether or not the locale should be in the '/{locale}/firefox/new/#download-fx' URL?

@pmclanahan
Copy link
Contributor

@jgmize @pmclanahan @flodolo So, if we enable all locales for /firefox/new/ as per PR #2215, is the LOCALES_WITH_TRANSITION list still relevant? See:

# Locals with transitional download pages

I think that's more a question for @pascalchevrel and possibly UX.

I wondered if it might make sense to send all downloaders, regardless of locale, to /firefox/new/#download-fx - even if they would see an en-US post-download page.

That sounds fine to me. Others might disagree.

Also, does PR #2215 affect whether or not the locale should be in the '/{locale}/firefox/new/#download-fx' URL?

I don't think so.

@jgmize
Copy link
Contributor

jgmize commented Aug 7, 2014

+1 for getting rid of LOCALES_WITH_TRANSITION

I wondered if it might make sense to send all downloaders, regardless of locale, to /firefox/new/#download-fx - even if they would see an en-US post-download page.

Makes sense to me.

Also, does PR #2215 affect whether or not the locale should be in the '/{locale}/firefox/new/#download-fx' URL?

Seems like we should send them to the URL without the locale to me.

@sgarrity
Copy link
Contributor Author

sgarrity commented Aug 7, 2014

Moving the discussion over to Bugzilla with a needsinfo for Pascal: https://bugzilla.mozilla.org/show_bug.cgi?id=874913#c17

@sgarrity
Copy link
Contributor Author

sgarrity commented Aug 8, 2014

Ok, we got some helpful feedback over on the bug. This PR now removes the LOCALES_WITH_TRANSITION and removes the local from the transition download page URL.

There are a bunch of download tests failing now, but I'm not sure I know how to deal with all of them. @jgmize Could I get a hand with those? Some may be obsolete, and other new tests may be warranted, but I just don't have the experience with python or bedrock tests to determine that myself. A few of them, at least, were testing for LOCALE in the download URL, which isn't relevant anymore (though those tests could be modified to still test the force_direct=true variation).

@jgmize
Copy link
Contributor

jgmize commented Aug 8, 2014

@sgarrity sure, I'll take on fixing the tests and push the results to a branch on the mozilla repo and put it on a demo server.

@jgmize jgmize mentioned this pull request Aug 12, 2014
@sgarrity
Copy link
Contributor Author

@jgmize has a better PR with these changes over in PR #2221.

@sgarrity sgarrity closed this Aug 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants