Spun off of bug 77080. This involves updating parser, nsLayoutDLF, nsContentDLF, possibly others.
Fixing this would fix a bunch of bugs... adding dependencies Reassigning to myself
17 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.1
are the fixes on track for landing in the next week or so? if not lets retarget.
The patch removes rft from parser and content. It does not touch CRtfDTD.cpp, which is not built anyway. It also does not remove it from the list of common mimetypes that the mime service will detect. We will now correctly bring up the downloading dialog for text/rtf, though. Reviews?
Keywords: patch, review
You should really ask Rick Gessner (email@example.com) about this, because I remember him saying in rtf bugs that he didn't want to remove rtf support.
ccing Rick. Rick, this change does not remove the RTF parser. It essentially just prevents us from trying to handle the text/rtf mimetype, which we don't anyway. If the RTF parser is ever at a point where it can be used well, we can reinstate the mimetype handling. What do you think?
I think rickg is on vacation. nistheeth/harishd?
The rtf code in mozilla is IMO there for no good reason, it only increases the complexity and size of the parser, I see no good reasons for having the code in the tree, and noone is actively maintaining the code either... IMO it should be removed, if we want it back it's always available in the cvs repository...
moving out to 0.9.2. I'd still like to get this in by Friday, though, if people think that this should happen and like what the patch does...
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Could we get a call one way or the other on this? To reiterate: as far as I can tell this patch removes all traces of rtf support in the parser except in files that are not part of the build. This is what we want, right? If not, could we decide what we _do_ want?
We want to cvs remove unused files as well. And remove all traces of RTF support in all the code (if there is no special handling for it it does not need to be mentioned anywhere).
With the patch in question, there are only five places text/rtf appears in the code in any form: 1) http://lxr.mozilla.org/seamonkey/source/htmlparser/src/MANIFEST#18 (this should be removed) 2) http://lxr.mozilla.org/seamonkey/source/htmlparser/src/nsParserCIID.h#69 (this can probably be removed) 3) http://lxr.mozilla.org/seamonkey/source/netwerk/mime/public/nsMimeTypes.h#119 (this is used by the next two) 4) http://lxr.mozilla.org/seamonkey/source/netwerk/mime/src/nsXMLMIMEDataSource.cpp#498 5) http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#102 (these include lists of common mime types such as text/rtf, application/postscript, and so on. What should we do with the text/rtf entries there? Once we decide on this I will attach an updated patch.) Whoever is checking in this patch should cvs remove htmlparser/src/CRtfDTD.* Comments on items 4 and 5?
Pushing out to 0.9.3. Can someone answer my question regarding points 4 and 5 in my comment from 2001-06-11 11:52 ?
Target Milestone: mozilla0.9.2 → mozilla0.9.3
I am pretty sure you can safely remove the RTF parts in 4) and 5) as well. We should treat RTF as just any other unknown mime type.
Boris: IMO, all traces of rtf should be yanked. CCing gagan,darin, and dougt for more comments.
Created attachment 39359 [details] [diff] [review] Updated patch that effaces all traces of text/rtf (and remaining traces of text/cpp)
a=tor for 0.9.2 checkin
My computer is not booting at the moment for unknown reasons. Could someone check this in please? Whoever does so, please also cvs remove htmlparser/src/CRtfDTD.*
This appears to have been checked in on the branch and trunk. Should this bug be closed?
Whiteboard: critical for 0.9.2 → critical for 0.9.2; checked in -should be closed?
Marking fixed, I also saw the checkin.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Sorry, forgot to close it. Yes, it was checked in.
Verified on build: 2001-06-27-04-Trunk platform: Win NT Marking it verified as per above developer comments.
You need to log in before you can comment on or make changes to this bug.