Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 411644 - "replace all" broken
Summary: "replace all" broken
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Editor (show other bugs)
Version: unspecified   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.0 M1   Edit
Assignee: Silenio Quarti CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-25 18:21 EDT by Manu Sridharan CLA
Modified: 2013-06-27 13:58 EDT (History)
3 users (show)

See Also:

Attachments
foo.js file to reproduce problem (3.43 KB, application/x-javascript)
2013-06-25 18:21 EDT, Manu Sridharan CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Manu Sridharan CLA 2013-06-25 18:21:27 EDT 
Created attachment 232759 [details]
foo.js file to reproduce problem

Open the attached file foo.js in Orion.  Press Cmd-F to open the find bar, type 'new Foo' as the "Find With" string, and 'new utils.Foo' as the "Replace With" string.  Then, click Replace All.

Expected: replacement occurs, and updated file contents display in the editor.
Actual: Editor contents do not change, though foo.js is shown as changed with the '*' indicator at the top.  If I save and reload foo.js, only the first occurrence of 'new Foo' has been replaced correctly.
Comment 1 Simon Kaegi CLA 2013-06-25 22:22:29 EDT 
Yep, in the console I get --

Uncaught Error: NotFoundError: DOM Exception 8 textView.js:5758
TextView._setDOMSelection textView.js:5758
TextView._updateDOMSelection textView.js:6353
TextView._setSelection textView.js:5955
TextView._modifyContent textView.js:5449
TextView.setText textView.js:2615
Editor.setText editor.js:321
Find._doReplace find.js:504
Find.replaceAll find.js:335
Comment 2 Simon Kaegi CLA 2013-06-25 22:24:38 EDT 
This didn't take a big file -- I did it with a file containing just 10 new Foo()s
Comment 3 libing wang CLA 2013-06-26 09:57:15 EDT 
I have similar functionality in the global search page.
I will consolidate some shared code for both and add more unite tests.
Also, we should not do a loop to simulate the replace step by step any more. We should change the text model once and refresh DOM tree.
Comment 4 Silenio Quarti CLA 2013-06-27 12:16:38 EDT 
I have fixed the exception in TextView. The replace all operation should finished properly now. 

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=b7ac443edfb08a2b72bfdd8a4684caf8777affa9

Libing, do you want to keep this bug open for the refactoring work or do you prefer to open a new one?
Comment 5 libing wang CLA 2013-06-27 13:21:32 EDT 
(In reply to comment #4)
> I have fixed the exception in TextView. The replace all operation should
> finished properly now. 
> 
> http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/
> ?id=b7ac443edfb08a2b72bfdd8a4684caf8777affa9
> 
> Libing, do you want to keep this bug open for the refactoring work or do you
> prefer to open a new one?

Thanks, Silenio.
Comment 6 libing wang CLA 2013-06-27 13:22:28 EDT 
I am closing this bug.
For the refactor work, I will add this as reminder in Bug 377317.
Comment 7 libing wang CLA 2013-06-27 13:27:27 EDT 
see bug 377317 commet 2.
Comment 8 libing wang CLA 2013-06-27 13:58:08 EDT 
The editor exception addressed in this bug was fixed but when I happened to verify it in another test case, it hits bug 411813. It is another issue anyway...