Remove old find code

VERIFIED FIXED in mozilla1.2alpha

Status

()

Core
Embedding: APIs
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Akkana Peck, Assigned: Akkana Peck)

Tracking

Trunk
mozilla1.2alpha
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

17 years ago
Once we're confident that the new find code is working well enough, we need to
remove the old find code, the pref, and the ugly switches in nsIWebBrowserFind.
(Assignee)

Comment 1

17 years ago
Target mozilla 1.0 for this.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0

Comment 2

17 years ago
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to adt@netscape.com.  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
(Assignee)

Updated

17 years ago
Target Milestone: mozilla1.2alpha → mozilla1.0.1
(Assignee)

Updated

17 years ago
Depends on: 141367
(Assignee)

Updated

17 years ago
Depends on: 141524
(Assignee)

Comment 3

17 years ago
Created attachment 81955 [details] [diff] [review]
Initial patch

Here is an initial patch for what needs to change to remove the old find code.

This bug is dependant on two others: there are dependencies on the old find
code in mailnews and in powerplant embedding.  Apparently the powerplant file
is obsolete and we need not worry about it, but mail is more problematical, and
may require that a mail person write a new dialog.  This patch disables find
within mail, which obviously isn't an acceptable solution but is enough to get
this to compile and show that the list of dependencies is approximately right.

I didn't make the removed files part of the patch.
Here is the list of files to be removed:

xpfe/components/find/resources/replacedialog.xul		   
xpfe/components/find/resources/replacedialog.js 
xpfe/components/find/resources/locale/en-US/replacedialog.dtd
editor/txtsvc/src/nsFindAndReplace.h
editor/txtsvc/src/nsFindAndReplace.cpp	 
xpfe/components/find/public/nsIFindComponent.idl
xpfe/components/find/src/nsFindComponent.h
xpfe/components/find/src/nsFindComponent.cpp
embedding/browser/powerplant/source/CFindComponent.h
embedding/browser/powerplant/source/CFindComponent.cpp
(Assignee)

Comment 4

16 years ago
No traction from mail people on bug 141524; bumping this one out, sigh.
Target Milestone: mozilla1.0.1 → mozilla1.2alpha
(Assignee)

Comment 5

16 years ago
Created attachment 89469 [details] [diff] [review]
Editor patch

Varada says that mail code is not used and can be safely removed.  So we can go
ahead with this patch.	Here's an updated version of the editor part of the
patch, including the removed files.
Attachment #81955 - Attachment is obsolete: true
(Assignee)

Comment 6

16 years ago
Created attachment 89470 [details] [diff] [review]
xpfe patch

Here's the xpfe part of the patch.  Reviews, anyone?

Comment 7

16 years ago
Comment on attachment 89469 [details] [diff] [review]
Editor patch

whoo hoo! this is great. sr=alecf with an editor-module reviewer
Attachment #89469 - Flags: superreview+

Comment 8

16 years ago
Comment on attachment 89470 [details] [diff] [review]
xpfe patch

I like this one even more! :) sr=alecf
Attachment #89470 - Flags: superreview+
(Assignee)

Comment 9

16 years ago
Kathy: do you have time to review this?

Comment 10

16 years ago
Comment on attachment 89469 [details] [diff] [review]
Editor patch

r=brade
Attachment #89469 - Flags: review+

Comment 11

16 years ago
Comment on attachment 89470 [details] [diff] [review]
xpfe patch

r=brade
Attachment #89470 - Flags: review+
(Assignee)

Comment 12

16 years ago
Fixed, yesterday.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 13

16 years ago
Although this bug is mentioned in CVS blame I can't find the patch that contains
the error that I want to describe here so maybe it was checked in my mistake?

Anyway the new error is this:
There are lots of places in editor code with this or a similar construct:

var htmlEditor = gEditor.QueryInterface(Components.interfaces.nsIHTMLEditor);
if (!htmlEditor) return;

Unfortunately in JS code QI throws on an error, it never returns null.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 14

16 years ago
Oh, and I'm seeing lots of JS errors in HTML Mail:
Error: gEditor has no properties
Source File: chrome://editor/content/editor.js
Line: 714
This message occurred 249 times during one message composition :-(
Also, multiple times on the console window:
An error occurred updating the cmd_smiley command
An error occurred updating the cmd_advancedProperties command
An error occurred updating the cmd_removeLinks command
An error occurred updating the cmd_removeNamedAnchors command
(Assignee)

Updated

16 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 15

16 years ago
Created attachment 90974 [details] [diff] [review]
back out changes accidentally checked in to editor.js

You're right!  That file should not have been checked in -- it's part of
another bug.  Sorry!

Here's a patch to revert that file.
(Assignee)

Comment 16

16 years ago
Bug 156918 apparently covers the backout, so I'm re-marking this fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 17

16 years ago
I looks like the (accidential?) checkin to editor.js also caused regression bug
156715.

Comment 18

16 years ago
Clean up verification of dated code change bus
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.