Turn on word-wrapping for plain text documents

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P2
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: JanH, Assigned: JanH)

Tracking

Firefox 66
Firefox 67
All
Android
Points:
---

Firefox Tracking Flags

(firefox66 wontfix, firefox67 fixed)

Details

Attachments

(6 attachments, 1 obsolete attachment)

For ages I've wondered why text files didn't word-wrap when viewed on Android and require far too much horizontal scrolling, and now I know why:
"plain_text.wrap_long_lines" was never enabled by default in mobile.js.

On top of that, we should probably make plaintext documents render with "width=device-width", so that the text appears at a readable size even without font inflation enabled, i.e. basically what I did for view-source in bug 1392996.

Just as bug 1392996 did for view-source documents, this will ensure that plain
text documents will always render with a readable font size by default on
mobile.

I want to turn on word-wrapping for plain text documents on mobile. However
currently, these are rendered with the desktop viewport by default, leading to
still tiny font sizes and still having to scroll horizontally if you then
pinch-zoom in to achieve a readable font size.

While font inflation would solve that problem, the layout of plain text
documents is so simple that we can also just render them using a
"width=device-width" viewport instead.

This test will test that plain text documents will be rendered as they would
include a <meta name="viewport" content="width=device-width"> tag.

Based on the current implementation of nsContentUtils::IsPlainTextType(), we
could just call that function again if we need to know whether we're
dealing with plain text content or not later on, but doing it this way ensures
we're always consistent with the current code in StartDocumentLoad(), which
includes some additional sanity checks.

Attachment #9042823 - Attachment description: Bug 1525662 - Part 2: Turn on word wrapping by default for plain text documents on mobile. r?snorp → Bug 1525662 - Part 5: Turn on word wrapping by default for plain text documents on mobile. r?snorp
Attachment #9046234 - Attachment description: Bug 1525662 - Part 4: Use "width=device-width" viewport for plain text documents. r?hsivonen → Bug 1525662 - Part 4: Use "width=device-width" viewport for plain text documents. r?botond
Attachment #9042822 - Attachment is obsolete: true

Thank you for working on this! The lack of this has bothered me on multiple occasions.

Attachment #9046233 - Attachment description: Bug 1525662 - Part 3: Provide a hook for treating missing <meta> viewport as "width=device-width". r?botond → Bug 1525662 - Part 3: Provide a hook for treating missing viewport specification as an explicit "width=device-width". r?botond

Comment 8

3 months ago
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/a8ec11730706
Part 1: Add test for viewport treatment of plain text documents. r=botond
https://hg.mozilla.org/integration/autoland/rev/f07e25fac5b2
Part 2: Remember plain-textness of HTML documents post loading. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/aaa8fbdbbe48
Part 3: Provide a hook for treating missing viewport specification as an explicit "width=device-width". r=botond
https://hg.mozilla.org/integration/autoland/rev/32249c69fee9
Part 4: Use "width=device-width" viewport for plain text documents. r=botond
https://hg.mozilla.org/integration/autoland/rev/1fa7091b4b4e
Part 5: Turn on word wrapping by default for plain text documents on mobile. r=snorp

Comment 9

3 months ago
Backout by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10f3bcf2a0ed
Backed out 6 changesets (bug 1525662, bug 1501665) for geckoview failures at org.mozilla.geckoview.test.NavigationDelegateTest.loadData_noMimeType. CLOSED TREE

It seems that that test is loading a 260 kB GIF as a text file (!). With word-wrapping turned on, we'll spend additional time reflowing and line-breaking the text, which on the ARM emulator is long enough to take us over the current timeout.

With word-wrapping turned on, loading that graphic as text takes around 7½ minutes in a debug build on the ARM emulator. Should I just increase the timeout on that test, should we use a smaller file there, or something else?

Flags: needinfo?(snorp)

(In reply to Jan Henning [:JanH] from comment #11)

With word-wrapping turned on, loading that graphic as text takes around 7½ minutes in a debug build on the ARM emulator. Should I just increase the timeout on that test, should we use a smaller file there, or something else?

Geez. My vote goes for a smaller (saner) file.

Flags: needinfo?(snorp)

The animation is cute, but one test is effectively treating the image data as a
text file, and displaying the whole file as text leads to very long loading
times on the ARM emulator when word-wrapping is turned on, especially with a
debug build.

Attachment #9042823 - Attachment description: Bug 1525662 - Part 5: Turn on word wrapping by default for plain text documents on mobile. r?snorp → Bug 1525662 - Part 6: Turn on word wrapping by default for plain text documents on mobile. r?snorp

Comment 14

2 months ago
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/65a5625c4790
Part 1: Add test for viewport treatment of plain text documents. r=botond
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc422efc3103
Part 2: Remember plain-textness of HTML documents post loading. r=hsivonen
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b61f5d7f7d2
Part 3: Provide a hook for treating missing viewport specification as an explicit "width=device-width". r=botond
https://hg.mozilla.org/integration/mozilla-inbound/rev/60e8006f70d2
Part 4: Use "width=device-width" viewport for plain text documents. r=botond
https://hg.mozilla.org/integration/mozilla-inbound/rev/7634907d87d0
Part 5: Use smaller image for Geckoview tests. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/14e0efdd3d9c
Part 6: Turn on word wrapping by default for plain text documents on mobile. r=snorp
You need to log in before you can comment on or make changes to this bug.