Closed
Bug 851230
Opened 11 years ago
Closed 11 years ago
Framed plain text documents are word wrapped since bug 253564 landed
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: jaws, Assigned: jaws)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
8.01 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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">
Assignee | ||
Updated•11 years ago
|
Keywords: regression
Assignee | ||
Updated•11 years ago
|
status-firefox22:
--- → affected
tracking-firefox22:
--- → ?
Assignee | ||
Comment 1•11 years ago
|
||
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.
Assignee | ||
Comment 2•11 years ago
|
||
Boris, do you know of anything that can be done here?
Flags: needinfo?(bzbarsky)
Comment 3•11 years ago
|
||
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)
Comment 4•11 years ago
|
||
Given the fact that bug 253564 would be a simple backout as Plan B, let's track
Assignee: nobody → jAwS
Assignee | ||
Comment 5•11 years ago
|
||
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)
Comment 6•11 years ago
|
||
No, I meant that the document would switch in the C++ code for plaintext documents, so we can make sure it happens before layout...
Comment 7•11 years ago
|
||
In particular, we can switch to the right stylesheet set _before_ we've seen the <link>, iirc, so can do it in StartDocumentLoad.
Assignee | ||
Updated•11 years ago
|
Attachment #726328 -
Flags: review?(felipc)
Attachment #726328 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 8•11 years ago
|
||
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 9•11 years ago
|
||
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+
Assignee | ||
Comment 10•11 years ago
|
||
Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/cc9e514a38a0
Status: NEW → ASSIGNED
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cc9e514a38a0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•