Opened 3 years ago

Closed 17 months ago

Last modified 17 months ago

#19226 closed Bug (fixed)

Admin readonly fields ignore newlines

Reported by: shadow Owned by: Melevir
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: atodorov@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

A TextField that is marked as readonly in the Admin will be displayed in a simple <p>, effectively removing any newlines that were entered. Since in HTML, <textarea> displays newlines as you enter them, it would be more appropriate if <pre> were used to display the data when it is readonly.

See: contrib/admin/templates/admin/includes/fieldset.html:17

thanks

Attachments (4)

ticket_19226.diff (693 bytes) - added by Melevir 3 years ago.
ticket_19226.2.diff (1.8 KB) - added by Melevir 3 years ago.
ticket_19226.3.diff (2.7 KB) - added by Melevir 3 years ago.
ticket_19226.4.diff (2.2 KB) - added by thiderman 3 years ago.
Patch with tests for br tags

Download all attachments as: .zip

Change History (21)

comment:1 Changed 3 years ago by shadow

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

A nicer solution (rather than <pre>) might be:

<p>{{ field.contents|linebreaksbr }}</p>

As this avoids the other styling that goes along with <pre>

comment:2 Changed 3 years ago by claudep

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 3 years ago by Melevir

  • Owner changed from nobody to Melevir

Changed 3 years ago by Melevir

comment:4 Changed 3 years ago by Melevir

  • Has patch set

comment:5 Changed 3 years ago by Melevir

  • Status changed from new to assigned

comment:6 Changed 3 years ago by claudep

  • Needs tests set

I think you could tweak the existing test_readonly_get in admin_views tests to include a new line in some value.

Changed 3 years ago by Melevir

comment:7 Changed 3 years ago by Melevir

  • Needs tests unset

comment:8 Changed 3 years ago by claudep

You are near, but you have tested a non-readonly field, otherwise the test would have failed (as you should get <br> instead of newlines).

Changed 3 years ago by Melevir

Changed 3 years ago by thiderman

Patch with tests for br tags

comment:9 Changed 3 years ago by thiderman

I fixed the last patch so that it actually executes and added an assertion that the conversion into <br /> tags was actually made.

comment:10 Changed 3 years ago by jezdez

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 3 years ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ec9d6b1122dd09168d8b2dbcabc745f22f0ef766:

Fixed #19226 -- Applied linebreaksbr to read-only fields in admin

Thanks shadow for the report, and Melevir and thiderman for the
patch.

comment:12 Changed 3 years ago by Claude Paroz <claude@…>

In 1b6e751430b6c151079f616ec5d4f8f3446ea24e:

[1.5.x] Fixed #19226 -- Applied linebreaksbr to read-only fields in admin

Thanks shadow for the report, and Melevir and thiderman for the
patch.
Backport of ec9d6b112 from master.

comment:13 Changed 3 years ago by anonymous

Yep guys,
I've found that this is not fixed for inlines. I've opened this ticket: #19429

comment:14 Changed 17 months ago by atodorov@…

  • Resolution fixed deleted
  • Status changed from closed to new

Hi guys,
this fix is breaking up display for me. I'm using computed read-only fields which display rich HTML into the model change form as a way to augment the administration experience which works really well for me.

Adding multiple line break tags breaks my display (this only happens because the templates used to render the fields have new lines in them). Now I have to go all-over my code and strip new lines before returning the field value.

It would be nice if the linebreaks filter skipped \n, <br/> replacement on safe strings.

comment:15 Changed 17 months ago by atodorov@…

  • Cc atodorov@… added

comment:16 Changed 17 months ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

Hi, could you please open a new ticket (referencing this one) with your use case instead of reopening a rather old ticket? And it would probably help understanding of your issue if you can provide some code example, maybe even screenshots.

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