Closed
Bug 313217
Opened 19 years ago
Closed 19 years ago
document.open('replace') now opens text/plain document where it used to open text/html
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: martijn.martijn, Assigned: bzbarsky)
References
()
Details
(Keywords: regression, testcase)
Attachments
(3 files)
644 bytes,
text/html
|
Details | |
1.87 KB,
patch
|
Details | Diff | Splinter Review | |
1.06 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
This was mentioned here: http://forums.mozillazine.org/viewtopic.php?t=326309&highlight= I'll attach a testcase. This used to work in 2004-05-20 build, but fails in 2004-05-22 build, so very likely a regression from bug 73409.
Reporter | ||
Comment 1•19 years ago
|
||
IE6 passes for this testcase.
Comment 2•19 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b5) Gecko/20051020 Firefox/1.5 ID:2005102014 Confirmed also works in Opera 8.5
![]() |
Assignee | |
Comment 3•19 years ago
|
||
This was purposeful, actually. Any type that's not text/html is treated as text/plain. Do Opera or IE actually replace in this case? Or just treat "replace" as a MIME type and fall back to HTML? I can do either one, assuming this is common enough for us to need to fix...
![]() |
Assignee | |
Comment 4•19 years ago
|
||
![]() |
Assignee | |
Comment 5•19 years ago
|
||
![]() |
Assignee | |
Comment 6•19 years ago
|
||
bc, do you think you can give me an idea of how common this is in time for 1.8?
Comment 7•19 years ago
|
||
http://lxr.mozilla.org/classic/source/lib/libmocha/lm_doc.c#1210 "replace" is an optional second argument. The first argument to document.open, if given, is always a MIME type. If not recognized, the default was "text/html", if I recall correctly. I did not try to wade through the classic netlib's uses of content_type to prove this. /be
![]() |
Assignee | |
Comment 8•19 years ago
|
||
Right. What I implemented is that we default to text/plain for unrecognized types, since rendering random data as HTML sounds like a recipe for disaster to me... Of course the only type we recognize is text/html. ;)
Comment 9•19 years ago
|
||
(In reply to comment #6) > bc, do you think you can give me an idea of how common this is in time for 1.8? Yes, I can try but I'm not sure how representative a scan would be of the full universe of pages. Would a scan of the top sites 1 level deep be enough? That will take about a day. A 2 deep scan would take several days.
![]() |
Assignee | |
Comment 10•19 years ago
|
||
Yeah, that's not going to be representative enough, probably. OK. So can someone tell me which of those two patches we want? That is, whether 'replace' in the first slot actually replaces in IE/Opera?
Reporter | ||
Comment 11•19 years ago
|
||
(In reply to comment #10) > That is, whether 'replace' > in the first slot actually replaces in IE/Opera? No, 'replace' in the first slot doesn't replace in IE6. Another history item in the list is added. But I see now that any random string other than 'replace' is doing the same as 'replace' in IE6. Only 'text/plain' is opening the document in text/plain.
![]() |
Assignee | |
Comment 12•19 years ago
|
||
Right. I don't think I want that behavior.
![]() |
Assignee | |
Comment 13•19 years ago
|
||
Comment on attachment 200309 [details] [diff] [review] Patch to make a type of "replace" mean HTML but not replace Just treat "replace" in the first slot as if nothing were set.
Attachment #200309 -
Flags: superreview?(jst)
Attachment #200309 -
Flags: review?(jst)
Comment 14•19 years ago
|
||
Comment on attachment 200309 [details] [diff] [review] Patch to make a type of "replace" mean HTML but not replace r+sr=jst
Attachment #200309 -
Flags: superreview?(jst)
Attachment #200309 -
Flags: superreview+
Attachment #200309 -
Flags: review?(jst)
Attachment #200309 -
Flags: review+
![]() |
Assignee | |
Updated•19 years ago
|
Assignee: general → bzbarsky
![]() |
Assignee | |
Comment 15•19 years ago
|
||
Fixed on trunk. Probably not worth messing with this on branch, but if someone thinks we should, please tell me why!
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
![]() |
Assignee | |
Updated•19 years ago
|
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
Comment 16•19 years ago
|
||
(In reply to comment #15) > Fixed on trunk. Probably not worth messing with this on branch, but if someone > thinks we should, please tell me why! > Hi wil this still fixed in Firefox 1.5 RC1 or Final too?
![]() |
Assignee | |
Comment 17•19 years ago
|
||
Firefox 1.5 is on the branch. So no, I have no plans to fix this in 1.5 unless this is a seriously major issue on the web.
Comment 18•19 years ago
|
||
(In reply to comment #17) > Firefox 1.5 is on the branch. So no, I have no plans to fix this in 1.5 unless > this is a seriously major issue on the web. > thnx for reply. i think that would be great if fix this in branch too, because in branch i had tested too, it fixed my another issue in YaBB 2 Software.This wil help my developer on www.YaBBforum.com
![]() |
Assignee | |
Comment 19•19 years ago
|
||
Again, if there is significant breakage from this, please describe it in detail. Right now, as things stand, I doubt drivers would take this on branch.
You need to log in
before you can comment on or make changes to this bug.
Description
•