Closed Bug 139420 Opened 23 years ago Closed 23 years ago

Replace buttons greyed out if replace field is empty

Categories

(SeaMonkey :: Composer, defect)

Product:
SeaMonkey
Component:
Composer
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: parish, Assigned: akkzilla)

References

Details

(Whiteboard: [adt3 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+] custrtm-)

Attachments

(1 file, 2 obsolete files)

New patch which fixes Replace All as well
1.92 KB, patch
akkzilla
: review+
kinmoz
: superreview+
chofmann
: approval+
Details | Diff | Splinter Review
In the Search dialogue the Replace, Replace and Find, and Replace All buttons remain greyed out until a character is entered in the Replace With field. Previously the buttons were enabled as soon as something was typed in the Search field. this means that you can no longer replace a string with a null string. IMO, the old behaviour was preferable because it allowed you to, for instance, remove all the quote characters when Forwarding a mail/news article that was badly mangled due to repeated quoting by entering "> " (without quotes) in the Find field, leaving the Replace With field empty, and clicking Replace All.
replacing with empty string is quite common....
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: nsbeta1
OS: Windows 2000 → All
Hardware: PC → All
Editor --> reassign
Product: MailNews → Browser
.
Assignee: naving → syd
Component: Search → Editor: Composer
QA Contact: laurel → sujay
--> akkana
Assignee: syd → akkana
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1alpha
how annoying; this should be adt2
Whiteboard: [adt2]
If it's ADT2, then it should also be moz 1.0. Patch coming.
Target Milestone: mozilla1.1alpha → mozilla1.0
Attached patch Fix (obsolete) — — Splinter Review
Comment on attachment 80652 [details] [diff] [review] Fix r=cmanske
Attachment #80652 - Flags: review+
Attached patch Better fix (obsolete) — — Splinter Review
Whoops! It turns out plaintext editor's InsertText refuses to do anything if the string passed in is null (not only does it not do anything, but it hangs rather than returning). So I need to work around that in the js. This patch fixes it.
Attachment #80652 - Attachment is obsolete: true
That only works for Replace. The attached patch fixes it for Replace All as well. Curiously, I tried your original patch when you posted it and it worked (with a nightly). The looping in InsertText appears to be new.
Comment on attachment 81579 [details] [diff] [review] Better fix Yes, good point -- thanks!
Attachment #81579 - Attachment is obsolete: true
Comment on attachment 81603 [details] [diff] [review] New patch which fixes Replace All as well Parish: I'll mark your new parts r=akkana ... can I assume I have your review on the rest of it?
Attachment #81603 - Flags: review+
Kin, can you sr this small replace dialog fix? Thanks!
Whiteboard: [adt2] → [adt2] Needs sr
Ref Comment 11: Err, yes, but do I have the authority to review code?
Doh! My previous comment should have been ref comment 12, not 11.
Comment on attachment 81603 [details] [diff] [review] New patch which fixes Replace All as well sr=kin@netscape.com Can we use the named constant |eNone| from nsIEditor instead of passing |0| to deleteSelection()?
Attachment #81603 - Flags: superreview+
Fixed on trunk. Let's let it bake there, then consider for branch.
IF this is fixed on trunk, how come its not marked RESOLVED-FIXED ?
Because I wasn't sure of the protocol for doing that. Is this right?
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: [adt2] Needs sr → [adt2] [FIXED_ON_TRUNK][Needs adt1.0.0+]
Keywords: adt1.0.0
nominate for rtm
Whiteboard: [adt2] [FIXED_ON_TRUNK][Needs adt1.0.0+] → [adt2 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+]
Keywords: nsbeta1 → nsbeta1+
Could we get verification on the trunk for this? Thanks.
Verified on the 05-17 trunk build. As soon as I type a character in the find field, all the buttons become active allowing me to replace whatever I am searching for with a null string.
Status: RESOLVED → VERIFIED
Whiteboard: [adt2 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+] → [adt2 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+] custrtm-
*** Bug 146649 has been marked as a duplicate of this bug. ***
Blocks: 143047
Keywords: adt1.0.0 → adt1.0.1+, mozilla1.0.1
Whiteboard: [adt2 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+] custrtm- → [adt3 rtm] [FIXED_ON_TRUNK][Needs adt1.0.0+] custrtm-
Comment on attachment 81603 [details] [diff] [review] New patch which fixes Replace All as well a=chofmann for 1.0.1
Attachment #81603 - Flags: approval+
*** Bug 148717 has been marked as a duplicate of this bug. ***
Fix checked in to the branch.
Keywords: fixed1.0.1
I just downloaded the 6/4 branch build for windows and I still see all the buttons dimmed out even though I left the first field blank and the second field I entered some text.. REOPEN.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
It's the other way around: you have to specify a match pattern, it's the second pattern (the replace string) that can be blank. It works for me in a 2002060407 linux build.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
ok...verified in 6/4 branch build
Status: RESOLVED → VERIFIED
Keywords: verified1.0.1
*** Bug 160485 has been marked as a duplicate of this bug. ***
Keywords: fixed1.0.1
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: