Remove our broken text/rtf support

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
HTML: Parser
P3
normal
VERIFIED FIXED
18 years ago
7 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla0.9.3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: critical for 0.9.2; checked in -should be closed?)

Attachments

(2 attachments)

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
Assignee: harishd → bzbarsky
Blocks: 45257, 45397, 76757
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.1

Comment 2

17 years ago
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

Comment 5

17 years ago
You should really ask Rick Gessner (rickg@netscape.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?

Comment 7

17 years ago
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.

Comment 15

17 years ago
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)
sr=jst

Comment 18

17 years ago
r=harishd

Comment 19

17 years ago
a=tor for 0.9.2 checkin
Whiteboard: critical for 0.9.2
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.*

Comment 21

17 years ago
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

Comment 23

17 years ago
Sorry, forgot to close it. Yes, it was checked in.

Updated

17 years ago
Status: RESOLVED → VERIFIED

Comment 24

17 years ago
Verified on
build: 2001-06-27-04-Trunk
platform: Win NT

Marking it verified as per above developer comments.

Updated

7 years ago
Depends on: 79864
You need to log in before you can comment on or make changes to this bug.