Page MenuHomePhabricator
Create Task
Maniphest T187454

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

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

SubjectRepoBranchLines +/-
Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1mediawiki/extensions/ProofreadPagewmf/1.31.0-wmf.21+143 -0
Fix .textSelection( 'encapsulateSelection', ... ) on #wpTextbox1mediawiki/extensions/ProofreadPagemaster+143 -0
Customize query in gerrit

Related Objects

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
Tpt mentioned this in T185917: Implement $.textSelection's replaceSelection and encapsulateSelection in NWE.Feb 15 2018, 3:28 PM
Tpt mentioned this in T187402: editTools and toolbar are not functional after adding the keyboard shortcuts in ProofreadPage.
Tpt moved this task from Backlog to Top priority on the ProofreadPage board.
Tpt added subscribers: Trizek-WMF, matmarex.Feb 15 2018, 3:33 PM
matmarex added a comment.Feb 15 2018, 6:25 PM

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.

gerritbot subscribed.Feb 15 2018, 7:22 PM

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

gerritbot added a project: Patch-For-Review.Feb 15 2018, 7:22 PM
gerritbot added a comment.Feb 15 2018, 7:47 PM

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

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

gerritbot added a comment.Feb 15 2018, 7:51 PM

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)

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-02-20 (1.31.0-wmf.22)).Feb 15 2018, 8:00 PM
Zdzislaw subscribed.Feb 15 2018, 8:03 PM
Jdforrester-WMF moved this task from To Triage to Current work on the VisualEditor board.Feb 15 2018, 8:31 PM
Jdforrester-WMF edited projects, added VisualEditor (Current work); removed VisualEditor.
matmarex claimed this task.Feb 15 2018, 8:59 PM
matmarex removed a project: VisualEditor-MediaWiki-2017WikitextEditor.
matmarex added a comment.Feb 15 2018, 9:23 PM

Scheduled a SWAT deployment for later today: https://wikitech.wikimedia.org/wiki/Deployments#deploycal-item-20180216T0000

Ankry subscribed.Feb 15 2018, 9:56 PM
Thurs subscribed.Feb 15 2018, 10:18 PM
Jdforrester-WMF moved this task from Incoming to Product owner review on the VisualEditor (Current work) board.Feb 15 2018, 11:59 PM
gerritbot added a comment.Feb 16 2018, 12:11 AM

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

Stashbot subscribed.Feb 16 2018, 12:16 AM

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.

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2018-02-13 (1.31.0-wmf.21)); removed MW-1.31-release-notes (WMF-deploy-2018-02-20 (1.31.0-wmf.22)).Feb 16 2018, 1:00 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL