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

Commit

Permalink
Merge pull request #27069 from asutherland/email-clone-fix
Browse files Browse the repository at this point in the history
Bug 1116087 - Safely clone elements for HTML cookie cache. r=jrburke
  • Loading branch information
asutherland committed Dec 31, 2014
2 parents 72dfe8a + 6d4e87b commit f90f714
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
5 changes: 4 additions & 1 deletion apps/email/js/cards/message_list.js
Expand Up @@ -1084,7 +1084,10 @@ return [
return;
}

var cacheNode = this.cloneNode(true);
// Safely clone the node so we can mutate the tree to cut out the parts
// we do not want/need.
var cacheNode =
htmlCache.cloneAsInertNodeAvoidingCustomElementHorrors(this);
cacheNode.dataset.cached = 'cached';

// Make sure toolbar is visible, could be hidden by drawer
Expand Down
38 changes: 35 additions & 3 deletions apps/email/js/html_cache.js
Expand Up @@ -3,6 +3,35 @@
'use strict';
define(function(require, exports) {

/**
* Safely clone a node so that it is inert and no document.registerElement
* callbacks or magic happens. This is not particularly intuitive, so it
* needs a helper method and that helper method needs an appropriately
* scary/warning-filled name.
*
* The most non-obvious thing here is that
* document.implementation.createHTMLDocument() will create a document that
* has the same custom element registry as our own, so using importNode
* on such a document will not actually fix anything! But a "template"
* element's contents owner document does use a new registry, so we use
* that.
*
* See the spec's details on this at:
* http://w3c.github.io/webcomponents/spec/custom/
* #creating-and-passing-registries
*/
exports.cloneAsInertNodeAvoidingCustomElementHorrors = function(node) {
// Create a template node with a new registry. In theory we could
// cache this node as long as we're sure no one goes and registers
// anything in its registry. Not caching it may result in slightly
// more GC/memory turnover.
var templateNode = document.createElement('template');
// content is a DocumentFragment which does not have importNode, so we need
// its ownerDocument.
var cacheDoc = templateNode.content.ownerDocument;
return cacheDoc.importNode(node, true); // yes, deep
};

/**
* Saves a JS object to document.cookie using JSON.stringify().
* This method claims all cookie keys that have pattern
Expand Down Expand Up @@ -56,9 +85,12 @@ exports.save = function htmlCacheSave(html) {
};

/**
* Serializes the node to storage. NOTE: it modifies the node tree,
* so pass use cloneNode(true) on your node if you use it for other
* things besides this call.
* Serializes the node to storage. NOTE: it modifies the node tree, and
* cloneNode(true) is *NOT SAFE* because of custom element semantics, so
* you must use cloneAsInertNodeAvoidingCustomElementHorrors(node) on
* your node and pass that to us. (And you call it instead of us because
* you probably really want to perform some transforms/filtering before you
* pass the node to us.)
* @param {Node} node Node to serialize to storage.
*/
exports.saveFromNode = function saveFromNode(node) {
Expand Down
4 changes: 3 additions & 1 deletion apps/email/js/mail_app.js
Expand Up @@ -161,7 +161,9 @@ function getStartCardArgs(id) {
'setup_account_info', 'immediate',
{
onPushed: function(cardNode) {
var cachedNode = cardNode.cloneNode(true);
var cachedNode =
htmlCache.cloneAsInertNodeAvoidingCustomElementHorrors(
cardNode);
cachedNode.dataset.cached = 'cached';
htmlCache.delayedSaveFromNode(cachedNode);
}
Expand Down

0 comments on commit f90f714

Please sign in to comment.