Ticket #2905 (closed Task: fixed)

Opened 6 years ago

Last modified 5 years ago

plugin:find and replace move to trunk

Reported by: garry.yao Owned by: martinkou
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

2905.patch (25.8 KB) - added by garry.yao 6 years ago.
2905_2.patch (25.6 KB) - added by garry.yao 6 years ago.
Remove some unnecessary lines of change
2905_3.patch (27.3 KB) - added by garry.yao 6 years ago.
2905_4.patch (24.5 KB) - added by garry.yao 6 years ago.
2905_5.patch (37.4 KB) - added by martinkou 5 years ago.
2905_6.patch (39.5 KB) - added by martinkou 5 years ago.

Change History

comment:1 Changed 6 years ago by garry.yao

  • Status changed from new to assigned
  • Owner set to garry.yao
  • Keywords Confirmed added

Changed 6 years ago by garry.yao

Changed 6 years ago by garry.yao

Remove some unnecessary lines of change

comment:2 Changed 6 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 6 years ago by garry.yao

comment:3 Changed 6 years ago by garry.yao

  • Keywords Review? added; HasPatch removed

Cleanning up codes.

comment:4 Changed 6 years ago by fredck

  • 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 6 years ago by garry.yao

comment:5 Changed 6 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 5 years ago by martinkou

  • Keywords Review- added; Review? removed
  • Status changed from assigned to new
  • Owner changed from garry.yao to martinkou

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 5 years ago by martinkou

comment:7 Changed 5 years ago by martinkou

  • Keywords Review? added; Review- removed

comment:8 Changed 5 years ago by martinkou

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 5 years ago by martinkou

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 5 years ago by fredck

  • 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 5 years ago by martinkou

comment:11 Changed 5 years ago by martinkou

  • Keywords Review? added; Review- removed

comment:12 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:13 Changed 5 years ago by martinkou

  • Status changed from new to closed
  • Resolution set to fixed

Fixed with [3902].

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