Skip to content

Commit c35f2e0

Browse files
committed
Fix adam-p#427: After rendering, text would be selected
This seemed to occur in Chrome on all pages, and in Firefox on some pages (Gmail, but not the contenteditable test page).
1 parent cb139e8 commit c35f2e0

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/common/CHANGES.md

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ Change Log
77
* [Fixed bug #435](https://github.com/adam-p/markdown-here/issues/435): On some pages, Markdown Here would spew cross-origin exceptions to the console. This was due to MDH trying to determine if a focused iframe-within-an-iframe was renderable.
88
- Thanks to [lincoln-b](https://github.com/lincoln-b) for reporting it.
99

10+
* [Fixed bug #427](https://github.com/adam-p/markdown-here/issues/427): In Chrome and Firefox (at least for some pages), after rendering the resulting text was selected.
11+
- Thanks to [nedchu](https://github.com/nedchu) for reporting it.
12+
1013

1114
2017-05-26: v2.13.1
1215
--------------------

src/common/markdown-here.js

+19-10
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ https://github.com/adam-p/markdown-here/issues/85
3838
;(function() {
3939

4040
"use strict";
41-
/*global module:false*/
41+
/* global module:false Utils */
4242

4343

4444
var WRAPPER_TITLE_PREFIX = 'MDH:';
@@ -207,8 +207,13 @@ function findSignatureStart(startElem) {
207207
return sig;
208208
}
209209

210-
// Replaces the contents of `range` with the HTML string in `html`.
211-
// Returns the element that is created from `html`.
210+
/**
211+
* Replaces the contents of `range` with the HTML string in `html`.
212+
* Returns the element that is created from `html`.
213+
* @param {Range} range
214+
* @param {string} html
215+
* @returns {Element}
216+
*/
212217
function replaceRange(range, html) {
213218
var documentFragment, newElement;
214219

@@ -225,10 +230,14 @@ function replaceRange(range, html) {
225230

226231
range.insertNode(documentFragment);
227232

228-
// Make sure the replacement is selected. This isn't strictly necessary, but
229-
// in order to make Chrome and Firefox consistent, we either need to remove
230-
// the selection in Chrome, or set it in Firefox. We'll do the latter.
231-
range.selectNode(newElement);
233+
// In some clients (and maybe some versions of those clients), on some pages,
234+
// the newly inserted rendered Markdown will be selected. It looks better and
235+
// is slightly less annoying if the text is not selected, and consistency
236+
// across platforms is good. So we're going to collapse the selection.
237+
// Note that specifying the `toStart` argument to `true` seems to be necessary
238+
// in order to actually get a cursor in the editor.
239+
// Fixes #427: https://github.com/adam-p/markdown-here/issues/427
240+
range.collapse(true);
232241

233242
return newElement;
234243
}
@@ -344,7 +353,7 @@ function hasParentElementOfTagName(element, tagName) {
344353
}
345354

346355

347-
// Look for valid raw-MD-holder element under `elem`. Only MDH wrappers will
356+
// Looks for valid raw-MD-holder element under `elem`. Only MDH wrappers will
348357
// have such an element.
349358
// Returns null if no raw-MD-holder element is found; otherwise returns that element.
350359
function findElemRawHolder(elem) {
@@ -396,8 +405,8 @@ function isWrapperElem(elem) {
396405
}
397406

398407

399-
// Find the wrapper element that's above the current cursor position and returns
400-
// it. Returns falsy if there is no wrapper.
408+
// Finds the wrapper element that's above the current cursor position and
409+
// returns it. Returns falsy if there is no wrapper.
401410
function findMarkdownHereWrapper(focusedElem) {
402411
var selection, range, wrapper = null;
403412

0 commit comments

Comments
 (0)