Page MenuHomePhabricator

Search and replace is totally broken when using it with ProofreadPage
Open, HighPublic

Description

Search and replace, as provided by the code Editor, just doesn't work when using it at "Page:" namespace. It does find the text, but selects (and replaces) text located about 50 characters to the right.

It is incredibly useful as you might have some easily fixable OCR error spanning multiple pages, and that tool is simple and incorporated by default. Most wikisourcians use other tools, such as TemplateScript, that includes search and replace functionality, but the fact that the defaul tool doesn't work (and causes havoc if used carelessly) is worrying.

STEPS TO REPRODUCE

  1. Go to any "Page:", say for example [ https://es.wikisource.org/w/index.php?title=P%C3%A1gina:Viajes_de_Fray_Francisco_Men%C3%A9ndez_a_Nahuelhuapi.pdf/232&action=edit ]
  2. Use the default search and replace tool to search for a text that you know it is there, for example "Tellez"
  3. Instead of "Tellez", the editor finds "añas, " (or some other text) (located 72 chars to the right)

NOTES

  1. I have tried this using both Chrome and Microsoft Edge, with the same results.
  2. I have tried this in several Wikisources, with the same resuts.
  3. I have tried this logged in and as a guest, with some changes: in the example given, it selects instead text located 39 chars to the right
  4. I have tried searching in other pages of the same work, and always selects text located the same number of characters away (is consistent within the work)
  5. Line breaks are probably counted as 2 characters (not fully tested)
  6. I have tried searching in other works, and each work has his own "number", different for logged in and guest, (for example 54 and 23 for [ https://es.wikisource.org/w/index.php?title=P%C3%A1gina:Mensaje_presidencial_del_21_de_mayo_de_2010.pdf/23&action=edit ]

Event Timeline

I have just done a quick investigation: the origin of this problem is that ProofreadPage overrides textarea.textSelection( 'getContents' ) too allow live preview but not the setSelection and encapsulateSelection methods that are now used by the WikiEditor find/replace system in order to work with syntax highlight. I believe that the proper fix for ProofreadPage is to overload properly these two methods.

This is a very serious bug, and a well-intentioned user can cause (reversible) damage to the pages, and frustration.
No one is going to even triage it?

@Tpt so why is it overriding the getSelection anyway ? which tools is this trying to support ?
Have we considered making the individual header/footer areas textSelection compatible instead ?

@TheDJ getSelection is overrided to support Wikitext to VE switch. We should definitely make header/footer compatible with textSelection. I have not taken time to implement this change yet. But if someone could write it, it would be much appreciated.

@Tpt ah, because the VE only knows about #wpTextbox1 so without this change it's not getting the version with the header and footer content...
hmm.. sounds like revisionslots/mcr cannot come quick enough ;)

T304303: Remove VE support (for now) is merged now, so the search-and-replace will start working again (later this week when the change is deployed).

This doesn't mean that we should give up on VE support, of course! But for now, these other bugs are more pressing.

It runs! thanks!

Alex

Il giorno dom 8 mag 2022 alle ore 01:59 Samwilson <
no-reply@phabricator.wikimedia.org> ha scritto:

Samwilson added a comment. View Task
https://phabricator.wikimedia.org/T183950

T304303: Remove VE support (for now)
https://phabricator.wikimedia.org/T304303 is merged now, so the
search-and-replace will start working again (later this week when the
change is deployed).

This doesn't mean that we should give up on VE support, of course! But for
now, these other bugs are more pressing.

*TASK DETAIL*
https://phabricator.wikimedia.org/T183950

*EMAIL PREFERENCES*
https://phabricator.wikimedia.org/settings/panel/emailpreferences/

*To: *Samwilson
*Cc: *Samwilson, OrbiliusMagister, Base, Yahya, Bodhisattwa,
Inductiveload, Jarnsax, Ruthven, Ankry, JAnD, Wargo, TheDJ, Candalua,
Alex_brollo, Arjunaraoc, Tpt, Aklapper, Ninovolador, Bebiezaza, Soda, MJL,
DannyS712, Matlin, Xover, Mahir256, Assassas77, Tshrinivasan, Info-farmer,
Hsarrazin, jayvdb, Shizhao, Billinghurst, jayantanth, Ltrlg, Krenair