Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Fix breaking out of <p> when deleting across paragraphs in FF #97

Merged
merged 23 commits into from
Apr 11, 2014
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"plumber-write": "~0.2.0",
"q": "~1.0.0",
"request": "~2.33.0",
"selenium-webdriver": "~2.37.0"
"selenium-webdriver": "^2.41.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caret, on the other hand, is more relaxed. It will update you to the most recent major version (the first number). ^1.2.3 will match any 1.x.x release including 1.3.0, but will hold off on 2.0.0.

http://fredkschott.com/post/2014/02/npm-no-longer-defaults-to-tildes/

},
"scripts": {
"test": "node test/runner",
Expand Down
4 changes: 2 additions & 2 deletions setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ npm install
bower install

# Download the selenium JAR
SELENIUM_VERSION=2.37.0
SELENIUM_VERSION=2.41.0
mkdir -p vendor
wget -O vendor/selenium-server-standalone-$SELENIUM_VERSION.jar \
https://selenium.googlecode.com/files/selenium-server-standalone-$SELENIUM_VERSION.jar
https://selenium-release.storage.googleapis.com/2.41/selenium-server-standalone-$SELENIUM_VERSION.jar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version is hardcoded in the URL?

3 changes: 0 additions & 3 deletions src/plugins/core/commands.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
define([
'./commands/indent',
'./commands/insert-html',
'./commands/insert-list',
'./commands/outdent',
'./commands/redo',
Expand All @@ -9,7 +8,6 @@ define([
'./commands/undo'
], function (
indent,
insertHTML,
insertList,
outdent,
redo,
Expand All @@ -22,7 +20,6 @@ define([

return {
indent: indent,
insertHTML: insertHTML,
insertList: insertList,
outdent: outdent,
redo: redo,
Expand Down
110 changes: 0 additions & 110 deletions src/plugins/core/commands/insert-html.js

This file was deleted.

119 changes: 119 additions & 0 deletions src/plugins/core/enforce-p-elements.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
define([
'lodash-modern/arrays/last',
'lodash-modern/collections/contains'
], function (
last,
contains
) {

/**
* Chrome and Firefox: Upon pressing backspace inside of a P, the browser
* deletes the paragraph element, leaving the scribe in a pristine state.
*
* Firefox: Erasing across multiple paragraphs, or outside of a
* whole paragraph (e.g. by ‘Select All’) will leave the scribe in a
* pristine state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could have an isolated case. Would you mind submitting one and adding it to BROWSERINCONSISTENCIES.md?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add more detail in this comment with regards to what you mean by “pristine state”. AFAIK, only the paragraph immediately following the caret is removed from it’s P element.

*
* Entering a new line in a pristine state state will insert
* `<div>`s (in Chrome) or `<br>`s (in Firefox) where previously we
* had `<p>`'s. This patches the behaviour of delete/backspace so
* that we do not end up in a pristine state.
*/

'use strict';


// TODO: not exhaustive?
var blockElementNames = ['P', 'LI', 'DIV', 'BLOCKQUOTE', 'UL', 'OL', 'H2'];
function isBlockElement(node) {
return contains(blockElementNames, node.nodeName);
}

/**
* Wrap consecutive inline elements and text nodes in a P element.
*/
function wrapChildNodes(parentNode) {
var groups = Array.prototype.reduce.call(parentNode.childNodes,
function (accumulator, binChildNode) {
var group = last(accumulator);
if (! group) {
startNewGroup();
} else {
var isBlockGroup = isBlockElement(group[0]);
if (isBlockGroup === isBlockElement(binChildNode)) {
group.push(binChildNode);
} else {
startNewGroup();
}
}

return accumulator;

function startNewGroup() {
var newGroup = [binChildNode];
accumulator.push(newGroup);
}
}, []);

var consecutiveInlineElementsAndTextNodes = groups.filter(function (group) {
var isBlockGroup = isBlockElement(group[0]);
return ! isBlockGroup;
});

consecutiveInlineElementsAndTextNodes.forEach(function (nodes) {
var pElement = document.createElement('p');
nodes[0].parentNode.insertBefore(pElement, nodes[0]);
nodes.forEach(function (node) {
pElement.appendChild(node);
});
});

parentNode._isWrapped = true;
}

// Traverse the tree, wrapping child nodes as we go.
function traverse(parentNode) {
var treeWalker = document.createTreeWalker(parentNode, NodeFilter.SHOW_ELEMENT);
var node = treeWalker.firstChild();

// FIXME: does this recurse down?

while (node) {
// TODO: At the moment we only support BLOCKQUOTEs. See failing
// tests.
if (node.nodeName === 'BLOCKQUOTE' && ! node._isWrapped) {
wrapChildNodes(node);
traverse(parentNode);
break;
}
node = treeWalker.nextSibling();
}
}


return function () {
return function (scribe) {

scribe.htmlFormatter.formatters.push(function (html) {
/**
* Ensure P mode.
*
* Wrap any orphan text nodes in a P element.
*/
// TODO: This should be configurable and also correct markup such as
// `<ul>1</ul>` to <ul><li>2</li></ul>`. See skipped tests.
// TODO: This should probably be a part of HTML Janitor, or some other
// formatter.
var bin = document.createElement('div');
bin.innerHTML = html;

wrapChildNodes(bin);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be a part of HTML Janitor? We already use that in a formatter. (See code comment above.)

traverse(bin);

return bin.innerHTML;
});

};
};

});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is in src/plugins/core. Should it be in src/plugins/core/formatters/html?

3 changes: 0 additions & 3 deletions src/plugins/core/patches.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@ define([
'./patches/commands/insert-html',
'./patches/commands/insert-list',
'./patches/commands/outdent',
'./patches/empty-when-deleting',
'./patches/events'
], function (
boldCommand,
indentCommand,
insertHTMLCommand,
insertListCommands,
outdentCommand,
emptyWhenDeleting,
events
) {

Expand All @@ -32,7 +30,6 @@ define([
insertList: insertListCommands,
outdent: outdentCommand
},
emptyWhenDeleting: emptyWhenDeleting,
events: events
};

Expand Down
81 changes: 0 additions & 81 deletions src/plugins/core/patches/empty-when-deleting.js

This file was deleted.

Loading