Support HTML in consent requirement descriptions

Review Request #9965 — Created May 22, 2018 and updated

Information

Djblets
release-1.0.x
312d6ff...

Reviewers

Instead of juggling mark_safe vs mark_safe_lazy and ugettext vs
ugettext_lazy and string interpolation, we now have a flag on the
BaseConsentRequirement that indicates whether or not the description
fields are HTML (in which case they will be rendered as-is and marked
safe) or text (in which case they will be wrapped with <p> tags). This
allows the rendering code (which is handled by a new
render_{field}_description method that can be overridden in a pinch to
specialize its behaviour) instead of naively by the template.

Used this with https://reviews.reviewboard.org/r/9967/

Description From Last Updated

django.utils.safestring.SafeText

chipx86chipx86

I mentioned this in a change making use of this, but it applies here. I don't think we should require …

chipx86chipx86

django.utils.safestring.SafeText

chipx86chipx86
brennie
Review request changed

Testing Done:

~  

Used this with https://reviews.reviewboard.org/r/9967/.

  ~

Used this with https://reviews.reviewboard.org/r/9967/

chipx86
  1. 
      
  2. djblets/privacy/consent/base.py (Diff revision 1)
     
     
    Show all issues

    django.utils.safestring.SafeText

  3. djblets/privacy/consent/base.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    I mentioned this in a change making use of this, but it applies here.

    I don't think we should require that text be set in the subclass to start with <p> if is_html is True. That should be handled here if no tags are present. In fact, we can go a step further and simplify this code in the process, and break the logic out so it's not duplicated.

    def _render_text(self, text, is_html):
        if is_html:
            text = mark_safe(text)
    
        text = conditional_escape(text)
    
        if not text.startswith('<') and not text.endswith('>'):
            text = format_html('<p>{0}</p>', text)
    
        return text
    

    This way, <p> is applied for the plain text case, and for the HTML case if outer tags aren't already present.

    It also properly handles safe text that's coming in (which allows the existing form we have where mark_safe is called -- honestly, we need to ship this code fast so I don't want to redo all the consent requirements again, and handling the existing case is going to be helpful there).

  4. djblets/privacy/consent/base.py (Diff revision 1)
     
     
    Show all issues

    django.utils.safestring.SafeText

  5. 
      
chipx86
  1. I forgot where we were with this. I remember postponing due to GDPR deadlines. Looks like some fixes were made but not published.

  2. 
      
Loading...