787673 - "Create Table from Selection" and "Find and Replace" no longer works in Composer and Thunderbird and other issues with deleteSelection

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[John Roenigk]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120826 Firefox/15.0 SeaMonkey/2.12
Build ID: 20120826214753

Steps to reproduce:

Selected a block of comma separated data from a csv file (as I have always done before v 2.12) and selected "Create Table from Selection".


Actual results:

Nothing.


Expected results:

It should have created a table

Comment 1

[Phoenix]

Confirming regression, working in
Build identifier: Mozilla/5.0 (Windows NT 5.2; rv:14.0) Gecko/20120715 Firefox/14.0.1 SeaMonkey/2.11
Broken on
User agent: Mozilla/5.0 (Windows NT 5.2; rv:15.0) Gecko/20120826 Firefox/15.0 SeaMonkey/2.12
Build identifier: 20120826214753
and trunk.
Can you test builds starting from http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2012/04/2012-04-25-00-30-46-comm-central-trunk/ from comm-central-trunk subfolders to determine, when the feature was broken? Just download seamonkey-2.12a1.en-US.win32.zip file, unpack it to separate folder and run from there

Comment 2

[Ian Neal]

2012-05-18-00-30-26-comm-central-trunk works
2012-05-20-00-30-31-comm-central-trunk fails
There was no build for 2012-05-19

Comment 3

[Ian Neal]

http://hg.mozilla.org/comm-central/pushloghtml?startdate=2012-05-18+00%3A30%3A26&enddate=2012-05-20+00%3A30%3A31
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-05-18+00%3A30%3A26&enddate=2012-05-20+00%3A30%3A31
Looks like it is in the Core Editor code

Comment 4

[Ian Neal]

cc'ing likely owner of suspect patches

Comment 5

[Ian Neal]

Looks like it was:
http://hg.mozilla.org/mozilla-central/rev/62ffb052629d
Bug 590640 part 5 - Delete empty wrappers when we delete the selection
Looks like there are 5 locations that need to be fixed:
http://mxr.mozilla.org/comm-central/ident?i=deleteSelection&tree=comm-central&filter=ui

Comment 6

[Ian Neal]

Fix required in Composer code (also shared with TB).

Comment 7

[Ian Neal]

Attachment: Add extra argument to deleteSelection calls [Checked in: Comment 9]

This patch adds the missing argument to the calls to deleteSelected on an editor. Used 0 / eStrip throughout.

Comment 8

[[e-mail]]

Comment on attachment 657616 [details] [diff] [review]
Add extra argument to deleteSelection calls [Checked in: Comment 9]

Eww, so this affects HTML source and find and replace (when the replacement is empty) too :-(

Using editor/gEditor.eNone/eStrip might work, it would make the code a little more obvious.

Comment 9

[Ian Neal]

Comment on attachment 657616 [details] [diff] [review]
Add extra argument to deleteSelection calls [Checked in: Comment 9]

Checked in with gEditor/editor.eNone/eStrip
http://hg.mozilla.org/comm-central/rev/1a52a6d4ea17

Comment 10

[Ian Neal]

Attachment: With eNone and eStrip [Checked in: trunk Comment 9 and c-a/c-b/c-r Comment 15]

Comment 11

[Ian Neal]

Comment on attachment 657712 [details] [diff] [review]
With eNone and eStrip [Checked in: trunk Comment 9 and c-a/c-b/c-r Comment 15]

[Approval Request Comment]
Regression caused by (bug #): Bug 590640
User impact if declined: Unable to use "Create Table from Selection" in message composition (for both SM and TB) and composer and find/replace with empty replacement string in composer.
Testing completed (on m-c, etc.): on c-c and locally
Risk to taking this patch (and alternatives if risky): None that I am aware of
String changes made by this patch: None

Comment 12

[Philip Chee]

Comment on attachment 657712 [details] [diff] [review]
With eNone and eStrip [Checked in: trunk Comment 9 and c-a/c-b/c-r Comment 15]

a=me

Comment 13

[Vincent (caméléon)]

Could you confirm if this fix could also solve Bug 787724 ? Is it possible to test it?

Comment 14

[Joe Sabash [:JoeS1]]

verified fixed using comma separated strings in:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Thunderbird/18.0a1 ID:20120903030554

Comment 15

[Ian Neal]

Comment on attachment 657712 [details] [diff] [review]
With eNone and eStrip [Checked in: trunk Comment 9 and c-a/c-b/c-r Comment 15]

http://hg.mozilla.org/releases/comm-aurora/rev/872fa48495cc
http://hg.mozilla.org/releases/comm-beta/rev/344511aabed7
http://hg.mozilla.org/releases/comm-release/rev/f884da2e73ac

Comment 17

[Mark Banner (:standard8)]

As this affects Thunderbird as well, this should really have gone into a MailNews component so that both apps where aware and can track.