Moodle
  1. Moodle
  2. MDL-19555

format_text() shortcut for NULL input

        Details

        • Type: Improvement Improvement
        • Status: Closed
        • Priority: Minor Minor
        • Resolution: Fixed
        • Affects Version/s: 1.9.5, 2.0
        • Fix Version/s: 2.0.3
        • Component/s: Performance
        • Difficulty:
          Easy
        • Affected Branches:
          MOODLE_19_STABLE, MOODLE_20_STABLE
        • Fixed Branches:
          MOODLE_20_STABLE

          Description

          While looking into MDL-19554, I've noticed that format_text() returns
          immediately if $text is '', but goes through full filtering/caching if
          $text is NULL. Perhaps it can take the same shortcut? TIA.

            Gliffy Diagrams

              Activity

              Ascending order - Click to sort in descending order
              Hide
              Permalink
              Eloy Lafuente (stronk7) added a comment -

              Assigning to David, sending to stable backlog and raising to critical hoping to get this fixed in next performance-focussed sprint.

              Note this needs fix both for 1.9.x and 2.0.x

              Thanks for the report!

              Show
              Eloy Lafuente (stronk7) added a comment - Assigning to David, sending to stable backlog and raising to critical hoping to get this fixed in next performance-focussed sprint. Note this needs fix both for 1.9.x and 2.0.x Thanks for the report!
              Hide
              Permalink
              Rossiani Wijaya added a comment -

              Thanks Matej for the patch.

              However, in this function, $text must always be set and its only checking for null value, therefore, I'm changing isset() to is_null().

              I will ask Sam to comment regarding this.

              diff patch (1.9): https://github.com/rwijaya/moodle/compare/MOODLE_19_STABLE...MDL-19555_m19
              diff patch (2.0): https://github.com/rwijaya/moodle/compare/master...MDL-19555_m20

              Thanks
              Rosie

              Show
              Rossiani Wijaya added a comment - Thanks Matej for the patch. However, in this function, $text must always be set and its only checking for null value, therefore, I'm changing isset() to is_null(). I will ask Sam to comment regarding this. diff patch (1.9): https://github.com/rwijaya/moodle/compare/MOODLE_19_STABLE...MDL-19555_m19 diff patch (2.0): https://github.com/rwijaya/moodle/compare/master...MDL-19555_m20 Thanks Rosie
              Hide
              Permalink
              Sam Hemelryk added a comment -

              Looks good thanks Rosie gets my +1

              Show
              Sam Hemelryk added a comment - Looks good thanks Rosie gets my +1
              Hide
              Permalink
              Rossiani Wijaya added a comment -

              Thanks Sam for reviewing.

              Submitted to pull request:
              1.9: Pull-424
              2.0: Pull-425

              Show
              Rossiani Wijaya added a comment - Thanks Sam for reviewing. Submitted to pull request: 1.9: Pull-424 2.0: Pull-425
              Hide
              Permalink
              Dongsheng Cai added a comment -

              Closing, Thanks

              Show
              Dongsheng Cai added a comment - Closing, Thanks
              Hide
              Permalink
              Helen Foster added a comment -

              Reopening so that a 2.0.3 fix version can be set. Also, let's wait until the weekly package is available before closing. Apologies for the extra notification emails.

              Show
              Helen Foster added a comment - Reopening so that a 2.0.3 fix version can be set. Also, let's wait until the weekly package is available before closing. Apologies for the extra notification emails.

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  1 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: