Closed Bug 1525662 Opened 6 years ago Closed 6 years ago

Turn on word-wrapping for plain text documents

Categories

(Firefox for Android Graveyard :: General, enhancement, P2)

Firefox 66
All
Android
enhancement

Tracking

(firefox66 wontfix, firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: JanH, Assigned: JanH)

Details

Attachments

(6 files, 1 obsolete file)

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
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
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
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
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: