• 
      

    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.