Page MenuHomePhabricator

WikiEditor headings dropdown inserts headings in wrong, random location in IE
Closed, ResolvedPublic

Description

Reproduced just now on English Wikipedia using IE8 on two different client PCs.

  1. Visit the wikipedia page http://en.wikipedia.org/wiki/Bug and edit it. (Any other article will do, but this report has part that are to the "Bug" article.)
  1. Click the Advanced menu to show the Heading dropdown
  1. Click inside textbox to put the cursor at the beginning of the first line
  1. In the Heading dropdown, choose "Heading 2" (or anything else)

What SHOULD happen: "== Heading text ==" should be inserted in the first line.

What DOES happen: the textbox cursor leaps down to line 26 and inserts "== Heading text ==" in the middle of a word, ending up like this:

  • [[Bug (2002 film)|''Bug'' (2002 film)]], a comedy-drama about a chain of events begun by a b

Heading text

oy squashing a bug


Version: unspecified
Severity: major
Platform: PC

Details

Reference
bz31847

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 21 2014, 11:58 PM
bzimport set Reference to bz31847.
bzimport added a subscriber: Unknown Object (MLST).

I am not logged in (anonymous) on Wikipedia, if that matters.

Another test case: Edit a blank article and choose "Level 2" from the Heading dropdown. "== Heading text ==" is inserted at the top, but the whole page scrolls downward at the same time, so the "== Heading text ==" is at the top of the browser window.

This problem does NOT happen if you highlight a line in the editor first, and then choose "Level 2". The appropriate heading is inserted with no scrolling.

This problem also does NOT happen with any buttons in the editor. Just dropdowns.

Works fine (no bug) in Firefox 7 and Chrome 14.

The problem seems to be true of all dropdowns in WikiEditor, including several custom ones I made.

I can repro this on a local trunk wiki in IE 7 and IE 8 on Windows XP SP3. No repro in IE 9 on Windows 7 -- seems fine there.

If I actually select a line, that line gets turned into a heading -- but if we just have a caret position but no selection, it jumps off to an unexpected line.

I see the same behavior happening on a 1.17 install, so this doesn't look like a new bug...

Took a quick peek in IE8's debugger...

When we run the $.textSelection('encapsulateText') call, in the IE path we end up going through the restoreCursorAndScrollTop function.

This pulls a scrollTop value of 0, and a pos (selection range) value of [368, 368], and forces the scrolltop and selection to those values. That character position comes out to the line where it's ending up inserting the header on my test document.

So question I guess is where that selection value's coming from. :)

The problem is caused by the call to context.fn.restoreCursorAndScrollTop() in
resources/jquery/jquery.textselection.js.

} else if ( document.selection && document.selection.createRange ) {
// IE

$(this).focus();
if ( context ) {
  context.fn.restoreCursorAndScrollTop();  ///////// This is the culprit
 }
 ....

This function is defined in WikiEditor/modules/jquery.wikiEditor.js and its
code looks suspicious :-)

'restoreCursorAndScrollTop': function() {

if ( $.client.profile().name === 'msie' ) {
  var IHateIE = context.$textarea.data( 'IHateIE' );
  if ( IHateIE ) {
    context.$textarea.scrollTop( IHateIE.scrollTop );
    context.$textarea.textSelection( 'setSelection', { start: IHateIE.pos[0],

end: IHateIE.pos[1] } );

    context.$textarea.data( 'IHateIE', null );
  }
}

},

Somehow this must be wrong for IE7 & IE8.

  • Bug 23579 has been marked as a duplicate of this bug. ***

The bad position is coming from getCaretPosition() in resources/jquery/jquery.textSelection.js.

In getCaretPosition(), it looks like the variable preRange contains the full text of the wiki page, not the textarea. I did an "alert(rawPreText)" and saw tons of text that appears elsewhere on the page but not in the textarea.

Ok, so we're getting the bad positioning by performing operations on bogus input in the classic IE path in textSelection's getCaretPosition().

It's fetching a selection via document.selection.createRange().duplicate(), then trying to figure out the start/end positions of the selection by making additional selections from the beginning and end of the textarea and extracting the bits around.

However, if there wasn't actually text selected... we get a totally empty TextRange. Its text property is empty, its boundingLeft/Top/Width/Height properties are all 0, etc.

The operations we perform on it end up reporting some totally bogus position inside the textarea, but it was GIGO the whole way.

Looks like we need a way to get caret position on IE 6/7/8 when there's no selection, and use that instead.

Added to QUnit test cases for jquery.textSelection in r100391.

Confirms that when we initially ask for the caret position we're getting some bogus value; if we have actually set an empty selection it'll read it back out though.

Setting focus on the textarea at top of getCaretPosition() seems to resolve this... though I'm not 100% sure it ought to be in there. At least calling it from the state-save ought to do the job though...

Went ahead in r100398 and added the focus() into getCaretPosition -- it's already in getSelection, presumably for much the same reasons. :)

This resolves the problems with the header drop-downs nicely, and doesn't appear to introduce any other problems I can see. Resolves the test case from r100391 as well.

Tagged for merge to 1.18/1.18wmf1.

Thank you Brion! I can confirm your patch works for our 1.17.0 installation.

If by any chance you are planning any 1.17.x patch releases, I suggest this as a good candidate for the patch.

This bug, or this fix, may be related to 32241 which has similar behavior.