Opened 10 years ago

Closed 10 years ago

#2905 closed Task (fixed)

plugin:find and replace move to trunk

Reported by: Garry Yao Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: General Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Review+ Cc:

Description

Find and replace plugin is required to move to trunk.

Attachments (6)

2905.patch (25.8 KB) - added by Garry Yao 10 years ago.
2905_2.patch (25.6 KB) - added by Garry Yao 10 years ago.
Remove some unnecessary lines of change
2905_3.patch (27.3 KB) - added by Garry Yao 10 years ago.
2905_4.patch (24.5 KB) - added by Garry Yao 10 years ago.
2905_5.patch (37.4 KB) - added by Martin Kou 10 years ago.
2905_6.patch (39.5 KB) - added by Martin Kou 10 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 10 years ago by Garry Yao

Keywords: Confirmed added
Owner: set to Garry Yao
Status: newassigned

Changed 10 years ago by Garry Yao

Attachment: 2905.patch added

Changed 10 years ago by Garry Yao

Attachment: 2905_2.patch added

Remove some unnecessary lines of change

comment:2 Changed 10 years ago by Garry Yao

Keywords: HasPatch added

This patch make the following changes to Martin's initial codes:

  1. Simplify the CharacterWalker by avoid using events internally, same reason as #2876.
  2. Make the pattern matchable on inline elements edge, e.g. <span>
  3. Introduce replace and replaceAll feature.
  4. Make search start right from pre-selected position.
  5. Add WholeWord match pattern validation based on find text pattern.
  6. Make find and search tabs share same values on common fields when switching between them.
  7. Fix a minor bug in _source/core/dom/document.js about createText

Martin, please give comments on it.

Changed 10 years ago by Garry Yao

Attachment: 2905_3.patch added

comment:3 Changed 10 years ago by Garry Yao

Keywords: Review? added; HasPatch removed

Cleanning up codes.

comment:4 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

This patch is proposing changes to "_source/plugins/find/dialogs/find.js", which is not available. I think this is not a patch for trunk.

Changed 10 years ago by Garry Yao

Attachment: 2905_4.patch added

comment:5 Changed 10 years ago by Garry Yao

Keywords: Review? added; Review- removed

Yes,I've aligned the patch with trunk, please go ahead with the reviewing.

comment:6 Changed 10 years ago by Martin Kou

Keywords: Review- added; Review? removed
Owner: changed from Garry Yao to Martin Kou
Status: assignednew

Still a number of problems:

  1. The dialog crashes in IE at opening.
  2. editor.lang.findAndReplace.replaceSuccMsg is missing in the language file.
  3. The dialog currently uses selectRanges() to highlight find results. This is not acceptable because it does not work in IE - sure the selection would still be visible in IE, but IE only allows one selection in each browser window so as soon as the user clicks on the dialog the highlight would be lost. I understand that such highlight logic would depend on #2768, which is not committed yet. But #2768's proposed patch is already working to some degree and it should be possible to propose a patch for the find dialog in conjunction with #2768's changes.
  4. The editor window does not scroll to the find results.

Since this feature is urgently needed, and I don't see you on Skype today, I'm taking over this ticket.

Changed 10 years ago by Martin Kou

Attachment: 2905_5.patch added

comment:7 Changed 10 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:8 Changed 10 years ago by Martin Kou

The replace function in the new patch crashes in IE, but after some investigations it's found to be a bug in the CKEDITOR.dom.range::extractContents() function.

Other features should be fully working.

comment:9 Changed 10 years ago by Martin Kou

The patch includes the changes in #2768 for making the find results highlight work. To remove those changes, simply remove all the changes in _source/plugins/styles/plugin.js.

comment:10 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
  • The following case is not working:
  1. Open the replace dialog.
  2. Type "a" into the find field.
  3. Type "AAA" into the replace field.
  4. Click "Replace" several times.

Note that when clicking replace, the editor correctly selects the next "a" occurrence, but it incorrectly starts this search at the previous selection position, instead of after the replacement text. It results in a endless replacement inside the replaced text.

  • Please change the alert message from "Successfully replaced %1 finds." to "%1 occurrence(s) replaced".
  • Is it possible to disable the "Match whole word" checkbox, instead of removing it when typing non words? Actually, I would leave it always available, because it makes sense to match "Chapter 3" and not "Chapter 30".
  • deleteContents should be used instead of extractContents. But it's still broken in IE. It's quite a serious break (content lost) so a fix for it must be included into the patch.

Changed 10 years ago by Martin Kou

Attachment: 2905_6.patch added

comment:11 Changed 10 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:12 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:13 Changed 10 years ago by Martin Kou

Resolution: fixed
Status: newclosed

Fixed with [3902].

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy