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

format_text() shortcut for NULL input

          Details

          • Type: Improvement
          • Status: Closed
          • Priority: 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

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  stronk7 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
                  stronk7 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
                  rwijaya 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
                  rwijaya 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
                  samhemelryk Sam Hemelryk added a comment -

                  Looks good thanks Rosie gets my +1

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

                  Thanks Sam for reviewing.

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

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

                  Closing, Thanks

                  Show
                  dongsheng Dongsheng Cai added a comment - Closing, Thanks
                  Hide
                  Permalink
                  tsala 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
                  tsala 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:
                        Fix Release Date:
                        5/May/11