Closed Bug 1514655 Opened 2 years ago Closed 2 years ago
Plaintext documents in subframes (or <object>) don't get word-wrapping
47 bytes, text/x-phabricator-request
|Details | Review|
So I'm not sure where this one should go, but let's start with the HTML parser. Enter this: data:text/html,<div style="width:200px;"><object data="https://s3-us-west-1.amazonaws.com/fftiers/out/text_RB.txt"></object></div> HTML created for rendering the object element contains: <html> <head> <link rel="alternate stylesheet" type="text/css" href="resource://content-accessible/plaintext.css" title="Wrap Long Lines"> </head> <body> <pre>Tier 1: Todd Gurley, Saquon Barkley, Ezekiel Elliott, Christian McCaffrey, Joe Mixon Tier 2: Nick Chubb, Phillip Lindsay, Alvin Kamara, Leonard Fournette, Dalvin Cook, David Johnson Tier 3: Justin Jackson, Chris Carson, Lamar Miller, Aaron Jones Tier 4: Tarik Cohen, Damien Williams, Derrick Henry, Jaylen Samuels, Sony Michel, Mark Ingram Tier 5: Tevin Coleman, James White, Gus Edwards, Jordan Howard Tier 6: Elijah McGuire, Marlon Mack, Doug Martin, Josh Adams Tier 7: Kenneth Dixon, Jeff Wilson, Matt Breida, Adrian Peterson, Kenyan Drake, Ito Smith Tier 8: Dion Lewis, Jalen Richard, Peyton Barber, Frank Gore, Theo Riddick </pre> </body> </html> We can see the link pointing to resource://content-accessible/plaintext.css But this not interpreted. Initially I thought there was a difference of implementations in between Blink/WebKit and Gecko, but in fact it's a difference just because Firefox seems to not load the CSS.
Summary: Horizontol scrollbar appears in div containing text → when object is called on a text file, the CSS is not applied plaintext.css
Are we supposed to load resource://content-accessible/plaintext.css cross-origin? In any case, this isn't the HTML parser.
Component: HTML: Parser → CSS Parsing and Computation
So this is somewhat weird. That sheet is loaded as an alternate stylesheet, so it shouldn't apply. Looks like for top-level documents we enable it here: https://searchfox.org/mozilla-central/rev/47edbd91c43db6229cf32d1fc4bae9b325b9e2d0/dom/html/nsHTMLDocument.cpp#755 But we intentionally don't do that for subframes (that's the IsChildOfSameType check), see bug 851230. We can change that, I guess... Is there any spec for any of this? Should there be?
See Also: → 851230
There is no spec. The relevant part of the spec is https://html.spec.whatwg.org/multipage/browsing-the-web.html#read-text and says: User agents may add content to the head element of the Document, e.g., linking to a style sheet, providing script, or giving the document a title. We could certainly switch to always applying plaintext.css, especially if all the other UAs have that behavior anyway.... I'm happy to review a patch that does that and removes the complexity.
Summary: when object is called on a text file, the CSS is not applied plaintext.css → Plaintext documents in subframes (or <object>) don't get word-wrapping
I'm happy to write that patch. You can disable wrapping in top-level documents anyway via View -> Page Style -> No style, so it seems like all the title stuff also doesn't make that much sense.
Assignee: nobody → emilio
I think having the title is nice in the sense of it being clear what exactly the sheet does... Though it looks like plaintext.css has grown some bidi stuff that is not line-wrapping related?
This matches other UAs.
Hey Jorg, do you know if thunderbird may rely on plain text documents not wrapping long text by any chance? This patch would change that.
What do you mean by "plain text documents"? All we render, as far as I know, are HTML documents. You won't change the behaviour of <pre> elements, I assume.
> What do you mean by "plain text documents"? Documents with type text/plain (or text/css, application/javacript, and a few others like that). > You won't change the behaviour of <pre> elements, I assume. Correct.
I'll land this, happy to provide a way for thunderbird to override this should they need one.
Backed out changeset 2eab9d9bd89e (Bug 1514655) for geckoview-junit perma failures NavigationDelegateTest.loadData_noMimeType CLOSED TREE Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&selectedJob=218330782&revision=2eab9d9bd89ecf6f26887f01c6df632ac1a255af Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=218330782&repo=autoland&lineNumber=3666 Backout push: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&revision=30d8fe076fd0347ef7fb9c1ee7332604ef265f88
Well, that's unexpected. This should have no effect on navigation unit tests... James, seems like you added this test, do you know how to run these, or who could help here? This is somewhat unexpected, but the only explanation I could find is that this GeckoView test was relying on the plaintext.css stylesheet not loading.
Flags: needinfo?(emilio) → needinfo?(snorp)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #12) > Well, that's unexpected. This should have no effect on navigation unit > tests... > > James, seems like you added this test, do you know how to run these, or who > could help here? This is somewhat unexpected, but the only explanation I > could find is that this GeckoView test was relying on the plaintext.css > stylesheet not loading. The test in question simply loads a gif as a data url with no mime type specified. It's timing out, which suggests that we are not getting either a location change or the page stop notification. Inspecting the logs indicates it's the latter, so I guess something in the patch broke that. You can run these locally with 'mach geckoview-junit`.  https://searchfox.org/mozilla-central/rev/9528360768d81b1fc84258b5fb3601b5d4f40076/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt#701
Alright, I'll give it a shot.
So I tried to do that. Got stuck for quite a while because adb timed out when using su 0. But my rooted phone has su -c. Yet the return code that the python stuff claimed to have was `1`. I ended up giving up and hacking around it with: diff --git a/testing/mozbase/mozdevice/mozdevice/adb.py b/testing/mozbase/mozdevice/mozdevice/adb.py index 01a3c1c516d8..0a8bf0d8fb16 100644 --- a/testing/mozbase/mozdevice/mozdevice/adb.py +++ b/testing/mozbase/mozdevice/mozdevice/adb.py @@ -625,6 +625,7 @@ class ADBDevice(ADBCommand): uid = 'uid=0' # Do we have a 'Superuser' sh like su? try: + self._have_su = True if (self._require_root and self.shell_output("su -c id", timeout=timeout).find(uid) != -1): self._have_su = True But the worst thing is that after so many snags `./mach -v geckoview-junit org.mozilla.geckoview.test.NavigationDelegateTest` passes for me locally with the patch applied of course. I'll try to rebase this patch and get this landed, but in the meantime, should I file a bug for that `su` thing? My setup is fairly simple. I have the latest magick installed (su -v gives 18.0:MAGISKSU (topjohnwu)).
Flags: needinfo?(emilio) → needinfo?(snorp)
You should try running against an emulator, since that's what automation is using. You can use 'mach android-emulator' to install and run the same emulator that is used in automation. It's failing on 4.3 opt, so you can use 'mach android-emulator --version 4.3' to get the right emulator. It's an ARM emulator, so it will be dramatically slower than a real device (or x86 emulator), so that may be part of the problem.
Webcompat Priority: --- → ?
Attachment #9032127 - Attachment description: Bug 1514655 - Always wrap plain text documents. → Bug 1514655 - Always wrap plain text documents. r=bzbarsky
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/5df5f0db2284 Always wrap plain text documents. r=bzbarsky
You need to log in before you can comment on or make changes to this bug.