Closed Bug 851230 Opened 7 years ago Closed 7 years ago

Framed plain text documents are word wrapped since bug 253564 landed

Categories

(Core :: Layout, defect)

22 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox22 + fixed

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Bug 253564 implemented word wrap for plain text documents, but it shouldn't have applied these changes to framed documents.

For example, load the following URL in Fx21 and Fx22:
data:text/html,<iframe src="http://people.mozilla.com/~jwein/longlines.txt">
Besides backing out bug 253564, I'm not sure what else can be done here.

I've looked and there doesn't appear to be a way within nsHtml5TreeBuilderCppSupplement.h or nsHtml5StreamParser.cpp to see if the document being loaded is within a frame.
Boris, do you know of anything that can be done here?
Flags: needinfo?(bzbarsky)
Well, one option is to always have the parser output an alternate stylesheet and then have the document switch to the relevant stylesheet set based on whether it's in a subframe and whether the pref is set....
Flags: needinfo?(bzbarsky)
Given the fact that bug 253564 would be a simple backout as Plan B, let's track
Assignee: nobody → jAwS
Attached patch Patch (obsolete) — Splinter Review
It should be noted that this will may have the effect of applying the stylesheet after layout.
Attachment #726328 - Flags: review?(felipc)
Attachment #726328 - Flags: review?(bzbarsky)
No, I meant that the document would switch in the C++ code for plaintext documents, so we can make sure it happens before layout...
In particular, we can switch to the right stylesheet set _before_ we've seen the <link>, iirc, so can do it in StartDocumentLoad.
Attachment #726328 - Flags: review?(felipc)
Attachment #726328 - Flags: review?(bzbarsky)
Attached patch Patch v2Splinter Review
I also removed the duplication within our codebase that checked if the content-type was plain text.
Attachment #726328 - Attachment is obsolete: true
Attachment #726831 - Flags: review?(bzbarsky)
Comment on attachment 726831 [details] [diff] [review]
Patch v2

> +  static bool IsPlainText(const nsAString& aContentType);

IsPlainTextType?

And perhaps document that it returns whether we render the type as plaintext?

That said, if all callers have an nsACString, why not make this method take that and avoid extra conversions?

>+    nsAString* titleCopy = new nsString(title);
>+    if (titleCopy)
>+      SetSelectedStyleSheetSet(*titleCopy);

Just:

  SetSelectedStyleSheetSet(title);

should work.

Also, why not put this code in nsHTMLDocument::StartDocumentLoad, where we already have plainText boolean?

r=me with those bits fixed.  Thanks!
Attachment #726831 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/cc9e514a38a0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
See Also: → 1514655
You need to log in before you can comment on or make changes to this bug.