Fix Bug 963816 - update privacy center #1637

Merged
merged 2 commits into from Apr 16, 2014

6 participants

@kyoshino
Mozilla member

Port files from PHP, add the Mozilla Privacy Policy, revise the Firefox Browser Privacy Notice, and make pages responsive.

@alexgibson alexgibson and 1 other commented on an outdated diff Jan 30, 2014
media/js/privacy/privacy.js
@@ -35,4 +35,35 @@ $(function() {
}
setDoNotTrackStatus();
+
+ // Accordion widgets in the highlight box
+ $('#main-content [role="tablist"]').each(function() {
+ var panel_open_text = $(this).data('panel-open-text');
+ var panel_close_text = $(this).data('panel-close-text');
+
+ if (navigator.userAgent.match(/MSIE\s[6-7]\./)) {
@alexgibson
Mozilla member

I haven't dug into testing/reviewing this PR, but if this code is for a vertical accordion may I suggest considering using mozilla-expanders.js? If it's suitable for the job, it has pretty good cross-browser support.

Also the layout issue in IE7 is likely the same one I encountered when coding the Contact/Spaces page, where we also use a vertical accordion for the menu. I managed to fix the height issue using the following selector:

#outer-wrapper {
    overflow: hidden;
}

This should force the browser to perform a layout when the page height resizes. It may just work.

@kyoshino
Mozilla member

I know mozilla-expanders.js has been used but there are problems: 1) it doesn't work if cookies are disabled, 2) no WAI-ARIA support. The privacy policy pages might be visited by privacy-conscious people, so I'd like to enable the accordion even on no-cookie environments. I'll later file a bug to fix those issues.

Thanks for the overflow: hidden tip! It worked fine (as always), but the behavior on IE7 is still odd: the expander content doesn't expand until the mouse pointer leaves the expander header. I think we could just disable the accordion on IE7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kyoshino
Mozilla member

Updated the sidebar blurbs as per comments in the bug. Could we put this on a demo server?

@kyoshino
Mozilla member

Updated the links on other pages also.

@kyoshino
Mozilla member

Added a tweak for in-product links. (Bug 961419)

@jpetto
Mozilla member

Unless there's sidebar content coming soon, I think on pages without a sidebar (e.g. /privacy/principles/), we should have the content extend to the full available width. Can probably just add a class to <div class="main-column">.

@jpetto jpetto and 1 other commented on an outdated diff Feb 7, 2014
bedrock/privacy/urls.py
+ page('/firefox/third-party', 'privacy/firefox/third-party.html'),
+ page('/firefox/2006-10', 'privacy/firefox/notice-2006-10.html'),
+ page('/firefox/2008-06', 'privacy/firefox/notice-2008-06.html'),
+ page('/firefox/2009-01', 'privacy/firefox/notice-2009-01.html'),
+ page('/firefox/2009-09', 'privacy/firefox/notice-2009-09.html'),
+ page('/firefox/2010-01', 'privacy/firefox/notice-2010-01.html'),
+ page('/firefox/2010-12', 'privacy/firefox/notice-2010-12.html'),
+ page('/firefox/2011-06', 'privacy/firefox/notice-2011-06.html'),
+ page('/firefox/2012-06', 'privacy/firefox/notice-2012-06.html'),
+ page('/firefox/2012-09', 'privacy/firefox/notice-2012-09.html'),
+ page('/firefox/2012-12', 'privacy/firefox/notice-2012-12.html'),
+ page('/firefox/2013-05', 'privacy/firefox/notice-2013-05.html'),
+ page('/firefox-os', 'privacy/firefox-os/notice.html'),
+ page('/thunderbird', 'privacy/thunderbird/notice.html'),
+ page('/websites', 'privacy/websites/notice.html'),
+ url(r'^/facebook/$', views.facebook, name='privacy/facebook'),
@jpetto
Mozilla member
jpetto added a note Feb 7, 2014

Any reason this couldn't just be page('/facebook', 'privacy/facebook/notice.html')? Would clean up views.py a little bit.

@kyoshino
Mozilla member
kyoshino added a note Feb 7, 2014

Looks like #411 changed it from page to url to add @xframe_allow.

@jpetto
Mozilla member
jpetto added a note Feb 7, 2014

Ah yes. Missed that - sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpetto jpetto commented on the diff Feb 7, 2014
bedrock/privacy/urls.py
@@ -8,21 +8,24 @@
from bedrock.privacy import views
urlpatterns = patterns('',
- page('/policies/firefox', 'privacy/firefox.html'),
- page('/policies/websites', 'privacy/websites.html'),
- page('/policies/thunderbird', 'privacy/thunderbird.html'),
- page('/policies/marketplace', 'privacy/marketplace.html'),
- page('/policies/persona', 'privacy/persona.html'),
- page('/policies/sync', 'privacy/sync.html'),
- page('/policies/test-pilot', 'privacy/test-pilot.html'),
- page('/policies/archive/firefox-octobe-2006', 'privacy/archive/firefox-october-2006.html'),
- page('/policies/archive/firefox-june-2008', 'privacy/archive/firefox-june-2008.html'),
- page('/policies/archive/firefox-january-2009', 'privacy/archive/firefox-january-2009.html'),
- page('/policies/archive/firefox-mobile-september-2009', 'privacy/archive/firefox-mobile-september-2009.html'),
- page('/policies/archive/firefox-january-2010', 'privacy/archive/firefox-january-2010.html'),
- page('/policies/archive/firefox-december-2010', 'privacy/archive/firefox-december-2010.html'),
@jpetto
Mozilla member
jpetto added a note Feb 7, 2014

Do we have/care about redirects for these /policies/archive/firefox-* URLs?

@kyoshino
Mozilla member
kyoshino added a note Feb 7, 2014

I think we don't have to care about it because those files are in Bedrock for a while but never published, IIUC. The archived files are still in the legacy PHP side.

@sgarrity
sgarrity added a note Feb 7, 2014

We've been told over on Bug 960689 that we may not need to port all of these, but if some (all?) of the work has been done already, I think it could be worthwhile.

@jpetto
Mozilla member
jpetto added a note Feb 7, 2014

Hm, you may be right in that they were never published. Might be worth asking Gareth to check GA to see if those URLs got any significant traffic?

@kyoshino
Mozilla member
kyoshino added a note Feb 8, 2014

Yeah, I have ported all archives from PHP; removing (some of) them is easy!

@kyoshino
Mozilla member
kyoshino added a note Feb 8, 2014

Those URL have been only in the urls.py settings. The live versions are still in PHP, so we can safely remove those.

BTW, octobe is a typo 😽

@jpetto
Mozilla member
jpetto added a note Feb 8, 2014

What do you mean? My favorite holiday is Hallowee. 🎃

😝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpetto jpetto and 1 other commented on an outdated diff Feb 7, 2014
media/css/privacy/privacy.less
+ border-bottom: 1px solid #ccc;
+ width: 80%;
+ }
+
+ [tabindex] {
+ outline: 0;
+ }
+
+ [role="tab"] {
+ &[aria-expaned] {
+ cursor: pointer;
+ }
+
+ a {
+ font-size: @baseFontSize;
+ font-family: 'Open Sans', sans-serif;
@jpetto
Mozilla member
jpetto added a note Feb 7, 2014

I think you could just use the .open-sans and .open-sans-light mixins instead, both here and below.

@kyoshino
Mozilla member
kyoshino added a note Feb 7, 2014

Cool, will use it 😺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpetto jpetto and 1 other commented on an outdated diff Feb 8, 2014
media/css/privacy/privacy.less
- }
- p {
- color: #fff;
- text-shadow: none;
- margin: 0;
- }
- img {
- display: block;
- background: @textColorPrimary;
+ }
+
+ input,
+ textarea {
+ -moz-box-sizing: border-box;
+ -webkit-box-sizing: border-box;
+ box-sizing: border-box;
@jpetto
Mozilla member
jpetto added a note Feb 8, 2014

We've got a mixin for this one, too - .border-box. 😄

@kyoshino
Mozilla member
kyoshino added a note Feb 8, 2014

Like it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpetto jpetto and 1 other commented on an outdated diff Feb 8, 2014
bedrock/privacy/views.py
@@ -36,7 +33,7 @@ def submit_form(request, form):
subject = 'Message sent from Privacy Hub'
sender = form.cleaned_data['sender']
to = ['privacy@mozilla.com']
@jpetto
Mozilla member
jpetto added a note Feb 8, 2014

Bug asks for this email address to be updated, but doesn't seem to specify a new one. Any idea? Maybe ask in the bug for a new address?

@kyoshino
Mozilla member
kyoshino added a note Feb 8, 2014

Good catch. I'll ask Mika about it in the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kyoshino kyoshino commented on the diff Feb 8, 2014
bedrock/privacy/urls.py
@@ -8,21 +8,24 @@
from bedrock.privacy import views
urlpatterns = patterns('',
- page('/policies/firefox', 'privacy/firefox.html'),
- page('/policies/websites', 'privacy/websites.html'),
- page('/policies/thunderbird', 'privacy/thunderbird.html'),
- page('/policies/marketplace', 'privacy/marketplace.html'),
- page('/policies/persona', 'privacy/persona.html'),
- page('/policies/sync', 'privacy/sync.html'),
- page('/policies/test-pilot', 'privacy/test-pilot.html'),
- page('/policies/archive/firefox-octobe-2006', 'privacy/archive/firefox-october-2006.html'),
@kyoshino
Mozilla member
kyoshino added a note Feb 8, 2014

octobe !! Do we mean 🐙 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpetto
Mozilla member

Last thing for tonight - just re-iterating that pages without a sidebar (/privacy/principles/, /legal/firefox/, and...?) should fill the full allowed width and not leave the sidebar area empty.

@kyoshino
Mozilla member

I'll make those pages to have a full-width content area. Thanks @jpetto @sgarrity Have a good weekend!

@kyoshino
Mozilla member

Addressed the style issues @jpetto pointed out. The Principles page now has a full-width content area (screenshot).

By the way, Mika asked us if it was possible for legal and vendors to maintain the docs and those translations on GitHub, like the Marketplace Developer Agreement. Is it okay to have the docs and translations under /media/ to load them from templates?

@m-alexis

It has been requested to put this on a demo server for stakeholder review before we push to production. Need to find a free one.

@kyoshino
Mozilla member
@kyoshino
Mozilla member

@jpetto @jgmize @pmclanahan Thanks for your help on the server error and sorry to bother you. I thought it was a good idea to load a simple HTML file, that will be maintained by legal, into a page and add attributes with PyQuery. I didn't notice PyQuery was only a requirement for the dev/test server. Do you have any concerns on using PyQuery on production?

@kyoshino
Mozilla member

Changed the email address as per requested. So this PR now includes everything, I believe.

@pmclanahan pmclanahan commented on an outdated diff Feb 27, 2014
requirements/prod.txt
@@ -11,6 +11,9 @@ GitPython==0.1.7
-e git://github.com/jsocol/commonware.git#egg=commonware
-e git://github.com/mozilla/nuggets.git#egg=nuggets
-e git://github.com/kurtmckee/feedparser#egg=feedparser
+pyquery==1.0
+markdown
@pmclanahan
Mozilla member

dev.txt installs prod.txt I'm pretty sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kyoshino
Mozilla member

Added mdx_outline and legal-docs to the submodules. I have tried to replace PyQuery with xml.dom.minidom but it's difficult to manipulate DOM trees without such a query selector. So I still would like to use it. PyQuery is already under vendor-local/lib/python/pyquery but @pmclanahan said we might have to install lxml (and its dependencies?) on the demo (and production?) server.

@pmclanahan
Mozilla member
@kyoshino
Mozilla member

Thank you @pmclanahan!

@jgmize
Mozilla member

While I'd rather just get lxml installed in our environments, if getting lxml installed turns out to be too big a pain point, I've had pretty good success with BeautifulSoup before. It supports a number of parsers, including html.parser from stdlib-- although the docs (and I) still recommend using lxml if it's an option, especially with older python versions like what we are still using in production (but hopefully not for much longer). The other pure-python parser you could use, either standalone or with BeautifulSoup, would be html5lib but while it may be more lenient than the parser from stdlib, it's probably also slower. The leniency probably isn't an issue for our site though, since we're pretty strict about producing correct html.

@pmclanahan
Mozilla member

I'd really suggest doing the markdown -> HTML -> modified HTML conversion during deployment. Adding dependencies shouldn't be as difficult since it would just be for the "admin node", and it would also make us care much less about speed, so perhaps we could more easily get by with a non lxml solution. This would be exactly as useful as far as I can tell since updating the Mardown source will require a prod push anyway.

@kyoshino
Mozilla member

Okay, I'll try to rewrite the code with BeautifulSoup + html.parser. By the way, doing something during deployment works now? #1333 has been blocked due to a commander script issue. And I'm not sure how to test such scripts.

@pmclanahan
Mozilla member

@kyoshino we run a bunch of stuff during deployment. I believe #1333 is broken for other reasons that we may not even know yet. But I'm pretty sure the fact that it's being done during deployment isn't the issue.

@kyoshino
Mozilla member

So I have replaced PyQuery with Beautiful Soup. Since there's no official repo on GitHub, I have put the library files under vendor-local/packages directly instead of adding it as a submodule.

Can someone push this to the demo1 server as requested in the bug? I'll implement a conversion command soon.

@kyoshino
Mozilla member

Because the markdown -> HTML -> modified HTML conversion won't take long, normal caches on the view might be enough rather than adding a complexity of management commands. Even if we go for a commander script, caches are needed to store modified HTML files. @pmclanahan @jgmize Thoughts?

@pmclanahan
Mozilla member

I was thinking we'd just write the modified HTML to the file system. Doing the conversion on the fly is possible, but seems a waste of CPU cycles. I doubt the page will be highly trafficked though...

@kyoshino
Mozilla member

I'll try to find the best way to store the modified HTML files then.

@kyoshino
Mozilla member

Implemented the command: ./manage.py generate_static_privacy_pages

@kyoshino
Mozilla member

Needs a spam prevention suggested in #1746.

@kyoshino
Mozilla member
  • Fixed a title overflow issue in Germany (Bug 980659)
  • Added {% set_lang_files "privacy/ffos_privacy" %} on privacy/notices/firefox-os.html for l10n backward compatibility but l10n is not activated...?
@kyoshino
Mozilla member

Hmm, set_lang_files doesn't affect get_lang_path() so privacy/notices/firefox-os.lang is needed with an ## active ## tag anyway.

@pmclanahan
Mozilla member
@kyoshino
Mozilla member

Re-added "learn more" links as per request in the bug.

@kyoshino
Mozilla member

Updated the command not to use Mock as @pmclanahan said on IRC.

@pmclanahan pmclanahan and 2 others commented on an outdated diff Mar 18, 2014
.../management/commands/generate_static_privacy_pages.py
+ '/privacy/firefox/': 'privacy/notices/firefox.html',
+ '/privacy/websites/': 'privacy/notices/websites.html',
+}
+
+
+class Command(NoArgsCommand):
+ help = 'Generate static HTML files for privacy notices.'
+
+ def handle_noargs(self, **options):
+ client = Client()
+ request = RequestFactory().get('/')
+ request.locale = settings.LANGUAGE_CODE
+
+ for url, template in TEMPLATES.iteritems():
+ # List the supported locales of the page
+ locales = get_translations(get_lang_path(template))
@pmclanahan
Mozilla member

Are these pages going to be translated? I didn't think the markdown content was going to be able to be translated.

@jgmize
Mozilla member
jgmize added a note Mar 18, 2014

I was under the impression that the wording needed to be very precise, including review by legal, which would preclude offering translations for non-technical reasons. If this is correct, I'd prefer to remove this part of the code.

@kyoshino
Mozilla member

Firefox OS Privacy Notice has already been localized into 8 languages. According to a bug comment, legal translations are done by a vendor, and the community reviews the final work.

The problem here is adding a new language requires a lang file in the SVN repo in addition to a translated markdown file on GitHub. It would be solved in a separate bug.

@jgmize
Mozilla member
jgmize added a note Mar 19, 2014

Awesome, I'm glad we're translating this. Thanks for the info, @kyoshino.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pmclanahan pmclanahan and 1 other commented on an outdated diff Mar 18, 2014
.../management/commands/generate_static_privacy_pages.py
+ for url, template in TEMPLATES.iteritems():
+ # List the supported locales of the page
+ locales = get_translations(get_lang_path(template))
+
+ for locale in locales.iterkeys():
+ cache_path = os.path.join(CACHE_DIR, locale, template)
+ cache_dir = os.path.dirname(cache_path)
+
+ # Create the destination directory first
+ if not os.path.exists(cache_dir):
+ os.makedirs(cache_dir)
+
+ # Save the rendered HTML as a static file
+ try:
+ f = open(cache_path, 'w')
+ f.write(client.get('/' + locale + url + '?cache=0').content)
@pmclanahan
Mozilla member

We don't need to cache the entire output of the page, just the processed markdown. The templates can just include the cached markdown. We should also not use anything designed for testing (like the test Client) in the production code. If you need to render a template you can do that more easily from the direct functions.

@pmclanahan
Mozilla member

Hmm.... after looking more closely at the templates I see why you did this. Hmmm... I don't think we need the l10n stuff, but since it's done it can't hurt. This method can work but we shouldn't use the Client like this. You should just call the view directly. I don't think it's a good idea to have a publicly accessible method for refreshing the cache. Adding a kwarg to the view functions and using that directly would be better I think.

@pmclanahan
Mozilla member

I could be wrong about the client thing too. @jgmize what do you think?

@jgmize
Mozilla member
jgmize added a note Mar 18, 2014

I'd also prefer to avoid using the test client like this, and as I mentioned above, if l10n is not going to happen for non-technical reasons, I'd prefer to have code that explicitly does not support it, and upon further reflection, a comment in the code as to the reasons for not supporting it would probably be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kyoshino
Mozilla member

Removing the test client... Still gets UndefinedError: 'gettext' is undefined when I run ./manage.py generate_static_privacy_pages though.

@kyoshino
Mozilla member

Updated the legal-docs. Still needs mozilla/legal-docs#27 and a fix for the UndefinedError though.

@kyoshino
Mozilla member

As chatted with @pmclanahan and @jgmize on IRC yesterday, I have removed the management command and added @cache_page to each view.

@pmclanahan pmclanahan and 1 other commented on an outdated diff Mar 25, 2014
bedrock/mozorg/helpers/misc.py
@@ -425,3 +431,83 @@ def releasenotes_url(release):
return reverse('firefox.os.releasenotes', args=[release.version])
else:
return reverse('firefox.releasenotes', args=(release.version, prefix))
+
+
+@jingo.register.function
+def load_legal_doc(request, doc_name):
@pmclanahan
Mozilla member

Looks like this no longer needs to be a helper?

@kyoshino
Mozilla member

Ah true, I'll move the function to privacy/views.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kyoshino
Mozilla member

Rebased, picked up the latest changes in legal-docs, and fixed broken redirects found by @chrismore:

RewriteRule ^/(\w{2,3}(?:-\w{2})?/)?privacy(.*)$ /b/$1privacy$2 [PT]

should be under

RewriteRule ^/(\w{2,3}(?:-\w{2})?/)?privacy/policies/(facebook|firefox-os|websites)/?$ /$1privacy/$2/ [L,R=301]
@kyoshino
Mozilla member

Updated my pull request to solve the remaining issues (see Bug 963816 Comment 101). Not squashed yet. Could I get a quick review? As per Mika's comment in the bug, they want this PR get merged to prod tomorrow, April 15.

@kyoshino
Mozilla member

The legal-docs submodule needs to be updated once the privacy-website-update branch is merged. It may be the last change.

@kyoshino
Mozilla member

I just picked up the latest changes to the legal-docs repo. Can someone give it a review? This should go to prod (today or) tomorrow.

@pmclanahan
Mozilla member

Do we need to just push this to demo1 and try it there?

@kyoshino
Mozilla member

Yeah, please push this to demo1 if possible. :)

@pmclanahan
Mozilla member

done

@kyoshino kyoshino referenced this pull request in mozilla/legal-docs Apr 15, 2014
Closed

What are these for: {: datetime="2014-03-26" } #57

@flodolo

I think I'm a bit lost on this. Are we going to import privacy docs directly from Github in .md format? Because right now we have several strings around SVN for Firefox OS Privacy (first locales like es and de use a .lang file, others a template).

@kyoshino
Mozilla member

Yeah, the legal-docs repo is included as a submodule in Bedrock, and the Markdown docs are loaded from there. The Firefox OS Privacy Notice page has a single string "Contact Us" other than the document body, and the string is in main.lang, so we only have to a .lang file with the ## active ## tag (to avoid redirecting to en-US). Now you can see the l10n and the language switcher as usual.

@kyoshino
Mozilla member

Yes, once this PR is deployed.

@jgmize jgmize merged commit c4ccb51 into mozilla:master Apr 16, 2014

1 check passed

Details default Jenkins build 'bedrock_github' #3818 has succeeded
@kyoshino kyoshino deleted the kyoshino:bug-963816-update-privacy-center branch Apr 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment