Moodle

format_text() shortcut for NULL input

Details

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

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

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: