Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-66104

Make HTMLPurifier handling of ‹nolink› tags to support them everywhere

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 3.6.2, 3.7, 3.8
    • 3.6.6, 3.7.2
    • Filters, General
    • MOODLE_36_STABLE, MOODLE_37_STABLE, MOODLE_38_STABLE
    • MOODLE_36_STABLE, MOODLE_37_STABLE
    • Hide

      Automated tests are enough to both verify that the expected behavior is now happening and that no regressions are introduced by the change.

      Show
      Automated tests are enough to both verify that the expected behavior is now happening and that no regressions are introduced by the change.

    Description

      While working on MDL-58680 tests, it was discovered that clean_text(), using the HTML Purifier... was breaking some content when <nolink> tags are used.

      So, for example, this html:

      <p>Hello, <nolink>world!</nolink></p>
      

      Is being converted to:

      <p>Hello, </p><nolink>world!</nolink>
      

      When the correct (expected) behavior, is to keep the html unmodified, because those tags should be allowed everywhere.

      Tracing back the problem it seems that @ MDL-24223, together with an HTMLPurifier upgrade, the behavior was changed:

      -        $this->addElement('nolink',  'Inline', 'Flow');
      +        $def->addElement('nolink', 'Block', 'Flow', array());
      

      Note that, surely, this has been unnoticed along all these years because we switched to the <span class="nolink> alternative for user-entered content in general long ago.

      But, irrespectively of that, the <nolink> ones should be working as expected because they can be used programmatically to skip filtering here and there (like we are trying to do @ MDL-58680) with the advantage of being removed before the final output happens.

      So, this issue is about to revert the handling of <nolink> tags back to "Inline" mode, that leads to the correct behavior of, simply, keeping HTML unmodified, without any early closing of tags (and cover the behavior with tests, to ensure it remains constant).

      Ciao

      Attachments

        Issue Links

          Activity

            People

              stronk7 Eloy Lafuente (stronk7)
              stronk7 Eloy Lafuente (stronk7)
              Andrew Lyons Andrew Lyons
              Jun Pataleta Jun Pataleta
              CiBoT CiBoT
              Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Ilya Tregubov, Laurent David, Raquel Ortega, Sara Arjona (@sarjona), Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Sujith Haridasan
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                9/Sep/19

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 1 hour, 53 minutes
                  1h 53m