Opened 9 years ago

Closed 9 years ago

#24045 closed Cleanup/optimization (fixed)

Useless calls to mark_safe and mark_for_escaping

Reported by: Aymeric Augustin Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently Django contains code similar to:

    if isinstance(message, SafeData):
        return mark_safe(message)

Since mark_safe is a no-op on SafeData instances, that's useless!

We should check when that code was introduced, if mark_safe was always a no-op on SafeData, and if that reveals a bug or simply useless code.

In the former case we should fix the bug, in the latter remove the code.

There are similar cases with mark_for_escaping which is a no-op both on SafeData and EscapeData instances.

Change History (3)

comment:1 by Tim Graham, 9 years ago

Has patch: set
Triage Stage: Unreviewed → Accepted

For 1, the code in question in trans_null.py was introduced in 64c0bf86775c5706355c9d3870345b8cff2c63f4. It looks like it could have been removed as part of 2779c299c8b7e4556b5285acc86d17560522e702. ​PR for that.

For 2, outside of the escape_filter, the only usage of mark_for_escaping seems to be in ​template/base.py which checks isinstance(obj, EscapeData) but calls mark_for_escaping(new_obj) so it doesn't seem redundant.

Let me know if I've missed something or should do further auditing.

comment:2 by Aymeric Augustin, 9 years ago

Triage Stage: Accepted → Ready for checkin

I looked again and the problem appears to be more narrow than I originally anticipated.

Thanks for doing the research!

comment:3 by Tim Graham <timograham@…>, 9 years ago

Resolution: → fixed
Status: new → closed

In 3325ec869c10aba0fcb52c826d79633818c14dbc:

Fixed #24045 -- Removed useless mark_safe() call in trans_null.py

Note: See TracTickets for help on using tickets.
Back to Top