Page MenuHomePhabricator

"encapsulateSelection" broken with the proofread-page content model
Closed, ResolvedPublic

Description

The encapsulateSelection method of jquery.textSelection is now inserting content in the wrong place when editing Page: pages on Wikisource.

The origin of this problem is that change [1] (T185917) is now using the getContents method of jquery.textSelection that is returning not only the text that is in the main textarea but also some extra wikitext for the header and footer. The introduction of such wikitext (useful when switching between VisualEditor and WikiEditor) is making the positions where the insertion should happen wrong, leading to introduce tags in the wrong places.

To reproduce this issue:

  1. Go to https://en.wikisource.org/w/index.php?title=Page:The_American_Indian.djvu/197&action=edit (it should load the WikiEditor)
  2. Select some text
  3. Click on the button to insert bold
  4. The bold tags quotes are not inserted in the right place.

[1] https://gerrit.wikimedia.org/r/c/408465/

Details

Related Gerrit Patches:
mediawiki/extensions/ProofreadPage : wmf/1.31.0-wmf.21Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1
mediawiki/extensions/ProofreadPage : masterFix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1

Event Timeline

Tpt created this task.Feb 15 2018, 3:25 PM
Restricted Application added a project: VisualEditor. · View Herald TranscriptFeb 15 2018, 3:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

So, ProofreadPage has a partial textSelection API override, and we now expect that if you're overriding stuff, then you should override all of getContents+setContents+getSelection+getCaretPosition+setSelection.

I see two ways to fix this:

  • Implement the missing methods in a consistent way (e.g. so that you can do .textSelection( 'setContents', '<noinclude><pagequality ...... /></noinclude>blah blah<noinclude></noinclude>' ) and it will parse this and split it across the three textboxes).
  • Copy-paste the encapsulateSelection implementation from core and just replace the .textSelection( 'setContents', ... )/.textSelection( 'getContents' ) calls in them with .val( ... )/.val().

I'm not sure how much effort would it take to do it the first way (the right way, if you ask me). I'll check that out and do it if it's easy. Otherwise I'll just do the second way.

Change 411061 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/ProofreadPage@master] Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1

https://gerrit.wikimedia.org/r/411061

Change 411061 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@master] Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1

https://gerrit.wikimedia.org/r/411061

Change 411066 had a related patch set uploaded (by Tpt; owner: Bartosz Dziewoński):
[mediawiki/extensions/ProofreadPage@wmf/1.31.0-wmf.21] Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1

https://gerrit.wikimedia.org/r/411066

Tpt added a comment.Feb 15 2018, 7:56 PM

@matmarex have made a change that fixes this bug and it is now merged in ProofreadPage master branch. It would be great to make it live on Wikisource (backported change: https://gerrit.wikimedia.org/r/411066)

Ankry added a subscriber: Ankry.Feb 15 2018, 9:56 PM
Thurs added a subscriber: Thurs.Feb 15 2018, 10:18 PM

Change 411066 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@wmf/1.31.0-wmf.21] Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1

https://gerrit.wikimedia.org/r/411066

Mentioned in SAL (#wikimedia-operations) [2018-02-16T00:16:52Z] <ebernhardson@tin> Synchronized php-1.31.0-wmf.21/extensions/ProofreadPage/modules/page/ext.proofreadpage.page.edit.js: SWAT: T187454 fix text selection on #wpTextbox1 (duration: 00m 58s)

matmarex closed this task as Resolved.Feb 16 2018, 12:25 AM
matmarex removed a project: Patch-For-Review.

Fixed and deployed.