1222586 - "Paste as first/last child" context menu doesn't work on SVG files

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Tim Nguyen :ntim]

TypeError: wc is undefined: trackSelection/</</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://react-devtools/main/trackSelection.js:39:9
EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:147:11
Selection.prototype.setNodeFront@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/selection.js:179:5
MarkupView.prototype.markNodeAsSelected@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:1285:7
MarkupView.prototype.navigate@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:760:5
MarkupContainer.prototype._onMouseDown@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:1895:5
EventListener.handleEvent*MarkupContainer.prototype.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:1698:5
MarkupElementContainer@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:2179:3
MarkupView.prototype.importNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:792:19
MarkupView.prototype.showNode@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:945:5
MarkupView.prototype._onNewSelection@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/markupview/markup-view.js:472:14
EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:147:11
Selection.prototype.setNodeFront@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/selection.js:179:5
InspectorPanel.prototype._deferredOpen/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:204:7
EventEmitter_once/handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:92:1
EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:147:11
InspectorPanel.prototype._onMarkupFrameLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:937:5
EventListener.handleEvent*InspectorPanel.prototype._initMarkup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:918:5
InspectorPanel.prototype._deferredOpen@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:197:5
InspectorPanel.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:99:14
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1236:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
InspectorPanel.prototype._getDefaultNodeForSelection/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:275:14
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1236:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
exports.WalkerFront<.querySelector<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:3287:12
InspectorPanel.prototype._getDefaultNodeForSelection/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:266:14
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1236:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1178:7
frontProto/</proto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/protocol.js:1338:14
exports.InspectorFront<.getPageStyle<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:3789:12
InspectorPanel.prototype._getPageStyle@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:227:12
InspectorPanel.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:95:14
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:451:5
InspectorPanel.prototype.open@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/inspector/inspector-panel.js:94:12
Toolbox.prototype.loadTool/onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1236:19
 event-emitter.js:152:0
Error writing response to: getUniqueSelector protocol.js:1025
<unavailable> protocol.js:907
Protocol error (unknownError): Error: undefined passed where a value is required inspector-panel.js:465
<unavailable> protocol.js:907

Comment 1

[Tim Nguyen :ntim]

STR:
Go to a SVG file (testcase url)
Copy the <path> as outer html
Try to paste it as last child of the <svg> root element

Comment 2

[Patrick Brosset <:pbro>]

Triaging (filter on CLIMBING SHOES).

Comment 3

[Sebastian Zartner [:sebo]]

Test case:
1. Open the Inspector on https://mdn.mozillademos.org/files/12668/MDN.svg
2. Right-click the <g id="g5461"> element and choose Copy > Outer HTML from the context menu
3. Right-click the <svg> tag and choose Paste > As Last Child from the context menu

=> The copied SVG doesn't get inserted and instead an error message is thrown.

With enabled e10s I only get this as error message:

<unavailable>   protocol.js:906

With disabled e10s I get this:

Error: Can't insert adjacent element to the root.
Stack trace:
WalkerActor<.insertAdjacentHTML@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/inspector.js:1993:13
generateRequestHandlers/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1042:19
onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1748:15
ChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7

The related condition is here:
https://hg.mozilla.org/mozilla-central/file/e9533997fcef/devtools/server/actors/inspector.js#l1994

Florent, that looks like your code, so I though maybe you could have a look at it.

Sebastian

Comment 4

[Florent Fayolle]

Oh, I see. The comment in the code says:
>    // Don't insert anything adjacent to the document element,
>    // the head or the body.
>    if (node.isDocumentElement()) {
>      throw new Error("Can't insert adjacent element to the root.");
>    }

This condition is relevant only for HTML documents, not for XML ones.

Two solutions :
- The simple one: don't raise exception if the user inserts a sibling of the HEAD or BODY elements.
- Or: raise an exception if the user inserts a sibling of the HEAD/BODY (as currently implemented), but support the insertion of elements as first child of the root of any XML documents (so support special cases when the document is an XML and non-XHTML one)

The first option seems to me the simplest, safest and the most compliant to how Element.prototype.insertAdjacentHTML behaves.

Thoughts? I'll take a look soon.

Florent

Comment 5

[Sebastian Zartner [:sebo]]

I'd also prefer the first solution. Note that the options to paste the copied structure are deactivated anyway, so this issue does not occur on HTML, anyway. And if people really want to add elements other than <head> and <body> to <html> is up to them (and already possible via drag & drop), IMO.

(In reply to Florent Fayolle from comment #4)
> Thoughts? I'll take a look soon.

If you want to take it, you can remove the need-info flag.

Sebastian

Comment 6

[Florent Fayolle]

I would like to take the bug, but I cannot assign it to myself. Thanks :)

Florent

Comment 7

[Sebastian Zartner [:sebo]]

(In reply to Florent Fayolle from comment #6)
> I would like to take the bug, but I cannot assign it to myself. Thanks :)

Did it for you. :-)

Sebastian

Comment 8

[Florent Fayolle]

Attachment: 1222586.patch

Thanks Sebastian!

:pbrosset is it useful to add a test for this?

Florent

Comment 9

[Tim Nguyen :ntim]

(In reply to Florent Fayolle from comment #8)
> Created attachment 8785680 [details] [diff] [review]
> 1222586.patch
> 
> Thanks Sebastian!
> 
> :pbrosset is it useful to add a test for this?
Yes, it would be nice, so we never break this again in the future.

Comment 10

[Patrick Brosset <:pbro>]

Comment on attachment 8785680 [details] [diff] [review]
1222586.patch

Review of attachment 8785680 [details] [diff] [review]:
-----------------------------------------------------------------

This seems sane to me. But, yes, please do add a test for this to avoid future regressions.

Comment 11

[Florent Fayolle]

Attachment: 1222586.patch

OK, here is the updated patch.

I took a name for the test file similar to the one for pasting HTML content (browser_inspector_menu-03-paste-items-svg.js VS browser_inspector_menu-03-paste-items.js). Is it OK for you?

Also I wonder if the test shouldn't rather be a unit test with mocking rather than testing the whole UI, as the test targets the behavior of a specific function. Are there stuffs in place that allows this?

Florent

Comment 12

[Tim Nguyen :ntim]

The patch is missing doc_svg_inspector_menu.svg

Comment 13

[Florent Fayolle]

Attachment: 1222586.patch

Thanks Tim!

Florent

Comment 14

[Patrick Brosset <:pbro>]

Comment on attachment 8787991 [details] [diff] [review]
1222586.patch

Review of attachment 8787991 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few final adjustments in the test and we're good to go!

::: devtools/client/inspector/test/browser.ini
@@ +33,5 @@
>    doc_inspector_search-suggestions.html
>    doc_inspector_search-svg.html
>    doc_inspector_select-last-selected-01.html
>    doc_inspector_select-last-selected-02.html
> +  doc_svg_inspector_menu.svg

Please rename such that it has the same prefix as all other support files in this folder:
doc_inspector_svg.svg for instance.

::: devtools/client/inspector/test/browser_inspector_menu-03-paste-items-svg.js
@@ +22,5 @@
> +var clipboard = require("sdk/clipboard");
> +registerCleanupFunction(() => {
> +  clipboard = null;
> +});
> +

nit: there are 2 empty new lines here. We only allow 1 in our eslint rules.

@@ +29,5 @@
> +  let { inspector, testActor } = yield openInspectorForURL(TEST_URL);
> +
> +  yield testPasteAdjacentHTMLMenu();
> +
> +  function* testPasteAdjacentHTMLMenu() {

Why use an extra nested function here? You could just put the whole body of the function inside the add_task body and avoid 1 level of indentation.

@@ +33,5 @@
> +  function* testPasteAdjacentHTMLMenu() {
> +    let refSelector = "svg";
> +    let oldHTML = yield testActor.getProperty(refSelector, "innerHTML");
> +    let nodeFront = yield getNodeFront(refSelector, inspector);
> +    yield selectNode(nodeFront, inspector);

selectNode accepts both nodeFronts and selectors, so you can save yourself the trouble of calling getNodeFront first. Also you can use getContainerForSelector instead of getContainerForNodeFront below.

@@ +36,5 @@
> +    let nodeFront = yield getNodeFront(refSelector, inspector);
> +    yield selectNode(nodeFront, inspector);
> +    let markupTagLine = getContainerForNodeFront(nodeFront, inspector).tagLine;
> +
> +    for (let { clipboardData, menuId } of PASTE_ADJACENT_HTML_DATA) {

You're not defining 'desc' and don't use it in the for loop. You probably just missed it.

@@ +51,5 @@
> +    }
> +
> +    let html = yield testActor.getProperty(refSelector, "innerHTML");
> +    let expectedHtml = PASTE_ADJACENT_HTML_DATA[0].clipboardData +
> +        oldHTML + PASTE_ADJACENT_HTML_DATA[1].clipboardData;

It feels a bit weird using a test array and then accessing items with index 0 and 1 at the end. Let me explain:
If you have a test array at the start of the test, it tells people that these are test cases, and that it's simple to add more test cases by adding things into the array later, if we need it in the future.
But here, you're relying on the 2 first items only to test something. So really, you shouldn't use an array I think, and instead just 2 const:

const PASTE_AS_FIRST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg" cx="42" cy="42" r="5"/>';
const PASTE_AS_LAST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg" cx="42" cy="42" r="15"/>';

And then remove the for loop, just paste the 2 strings one by one, and finally test the expected html is:

PASTE_AS_FIRST_CHILD + oldHTML + PASTE_AS_LAST_CHILD

Comment 15

[Tim Nguyen :ntim]

Comment on attachment 8787991 [details] [diff] [review]
1222586.patch

Review of attachment 8787991 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/test/doc_svg_inspector_menu.svg
@@ +7,5 @@
> +   xmlns="http://www.w3.org/2000/svg"
> +   id="svg3624"
> +   height="200"
> +   width="226.85512"
> +   version="1.1">

Please run this SVG through SVGO: https://jakearchibald.github.io/svgomg/
Make sure the Prettify code option is on.

Comment 16

[Florent Fayolle]

Attachment: 1222586.patch

(In reply to Patrick Brosset <:pbro> from comment #14)
> Comment on attachment 8787991 [details] [diff] [review]
> 1222586.patch
> 
> Review of attachment 8787991 [details] [diff] [review]:
> -----------------------------------------------------------------

> 
> @@ +29,5 @@
> > +  let { inspector, testActor } = yield openInspectorForURL(TEST_URL);
> > +
> > +  yield testPasteAdjacentHTMLMenu();
> > +
> > +  function* testPasteAdjacentHTMLMenu() {
> 
> Why use an extra nested function here? You could just put the whole body of
> the function inside the add_task body and avoid 1 level of indentation.


Very true! Removed.

> 
> @@ +33,5 @@
> > +  function* testPasteAdjacentHTMLMenu() {
> > +    let refSelector = "svg";
> > +    let oldHTML = yield testActor.getProperty(refSelector, "innerHTML");
> > +    let nodeFront = yield getNodeFront(refSelector, inspector);
> > +    yield selectNode(nodeFront, inspector);
> 
> selectNode accepts both nodeFronts and selectors, so you can save yourself
> the trouble of calling getNodeFront first. Also you can use
> getContainerForSelector instead of getContainerForNodeFront below.

Done.


> 
> @@ +51,5 @@
> > +    }
> > +
> > +    let html = yield testActor.getProperty(refSelector, "innerHTML");
> > +    let expectedHtml = PASTE_ADJACENT_HTML_DATA[0].clipboardData +
> > +        oldHTML + PASTE_ADJACENT_HTML_DATA[1].clipboardData;
> 
> It feels a bit weird using a test array and then accessing items with index
> 0 and 1 at the end. Let me explain:
> If you have a test array at the start of the test, it tells people that
> these are test cases, and that it's simple to add more test cases by adding
> things into the array later, if we need it in the future.
> But here, you're relying on the 2 first items only to test something. So
> really, you shouldn't use an array I think, and instead just 2 const:
> 
> const PASTE_AS_FIRST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg"
> cx="42" cy="42" r="5"/>';
> const PASTE_AS_LAST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg"
> cx="42" cy="42" r="15"/>';
> 
> And then remove the for loop, just paste the 2 strings one by one, and
> finally test the expected html is:
> 
> PASTE_AS_FIRST_CHILD + oldHTML + PASTE_AS_LAST_CHILD

Oh yeah very good point, thanks!

> Please run this SVG through SVGO: https://jakearchibald.github.io/svgomg/
> Make sure the Prettify code option is on.

Oh yes, there was a lot of noise in this file. That's done, thanks, although I get some obscure exception now:

> console.error:
>   Error writing response to: getUniqueSelector
> console.error:
>   Message: Error: undefined passed where a value is required
>   Stack:
>     identityWrite@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools
> /shared/protocol.js:109:11
> RetVal<.write@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/sha
> red/protocol.js:528:12
> Response<.write/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools
> /shared/protocol.js:681:16
> …

Not sure whether that's bad or not, as it doesn't prevent the test to pass nor to fail. What do you think?

Florent

Comment 17

[Tim Nguyen :ntim]

Comment on attachment 8788604 [details] [diff] [review]
1222586.patch

Review of attachment 8788604 [details] [diff] [review]:
-----------------------------------------------------------------

You forgot to `hg add` SVG file :) I recommend using `hg rename` when you rename files btw.

::: devtools/client/inspector/test/browser_inspector_menu-03-paste-items-svg.js
@@ +7,5 @@
> +
> +const TEST_URL = URL_ROOT + "doc_inspector_svg.svg";
> +const PASTE_AS_FIRST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg" cx="42" cy="42" r="5"/>';
> +const PASTE_AS_LAST_CHILD = '<circle xmlns="http://www.w3.org/2000/svg" cx="42" cy="42" r="15"/>';
> +const PASTE_ADJACENT_HTML_DATA = [

Looks unused, please remove this (Eslint usually catches this btw).

@@ +20,5 @@
> +];
> +
> +var clipboard = require("sdk/clipboard");
> +registerCleanupFunction(() => {
> +  clipboard = null;

So instead of doing this, I recommend making clipboard a local `let` variable inside the body of add_task();

Comment 18

[Florent Fayolle]

Attachment: 1222586.patch

Many thanks Tim for the tips! Here is the patch updated.

Florent

Comment 19

[Patrick Brosset <:pbro>]

Comment on attachment 8788758 [details] [diff] [review]
1222586.patch

Review of attachment 8788758 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!

::: devtools/client/inspector/test/browser_inspector_menu-03-paste-items-svg.js
@@ +26,5 @@
> +  yield pasteContent("node-menu-pastelastchild", PASTE_AS_LAST_CHILD);
> +
> +  let html = yield testActor.getProperty(refSelector, "innerHTML");
> +  let expectedHtml = PASTE_AS_FIRST_CHILD + oldHTML + PASTE_AS_LAST_CHILD;
> +  ok(html === expectedHtml);

Please use `is` instead of `ok` in these cases:
is(html, expectedHtml, "The new innerHTMLof the SVG node is correct");

Comment 20

[Tim Nguyen :ntim]

Comment on attachment 8788758 [details] [diff] [review]
1222586.patch

Review of attachment 8788758 [details] [diff] [review]:
-----------------------------------------------------------------

Agreed with pbro on using is instead of ok. Make sure you push this to try.

::: devtools/client/inspector/test/browser_inspector_menu-03-paste-items-svg.js
@@ +11,5 @@
> +
> +add_task(function* () {
> +  let clipboard = require("sdk/clipboard");
> +  registerCleanupFunction(() => {
> +    clipboard = null;

You should be able to remove the cleanup function. The clipboard var is only available within the body of add_task, so it will get cleaned up automatically anyway.

::: devtools/client/inspector/test/doc_inspector_svg.svg
@@ +1,1 @@
> +<?xml version="1.0" encoding="UTF-8"?>

Can you remove this line?

Comment 21

[Florent Fayolle]

Attachment: 1222586.patch

I took your remarks into account. 

Could you try-push for me? My account has been disabled since my last contribution (if I resume contributing more, I will ask for a reactivation).

Thanks!

Florent

Comment 22

[Patrick Brosset <:pbro>]

Comment on attachment 8789039 [details] [diff] [review]
1222586.patch

Review of attachment 8789039 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. I gave this one last test and it worked fine.
Just one comment about the test, but no need to ask for another review after this.
Also, another comment about the commit message: it should not just state the bug title, it should explain what you actually fixed.
Maybe something like Bug 1222586 - Don't assume document to be HTML when pasting markup into the markup-view; r=pbro
Or something that you feel makes more sense.

I've pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cba8f0fb769

::: devtools/client/inspector/test/browser_inspector_menu-03-paste-items-svg.js
@@ +2,5 @@
> +/* Any copyright is dedicated to the Public Domain.
> +http://creativecommons.org/publicdomain/zero/1.0/ */
> +"use strict";
> +
> +// Test that different paste items work in the context menu

nit: please update this comment. It should say that this tests that HTML can be pasted in SVG elements.

Comment 23

[Tim Nguyen :ntim]

Comment on attachment 8789039 [details] [diff] [review]
1222586.patch

Review of attachment 8789039 [details] [diff] [review]:
-----------------------------------------------------------------

pbro's r+ should be enough :) Try is green, so feel free to mark checkin-needed once you've addressed Patrick's comment.

Comment 24

[Florent Fayolle]

Attachment: 1222586.patch

Thank you very much Patrick, Tim!

@Patrick I took into account your last comments. I mark the bug as checkin-needed as Tim suggests.

Florent

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cd2f33114ac4
Don't assume document to be HTML when pasting markup into the markup-view; r=pbro

Comment 26

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cd2f33114ac4

Comment 27

[Sebastian Zartner [:sebo]]

Works fine for me. Merci beaucoup, Florent ! :-)

Sebastian

Comment 28

[Florent Fayolle]

Bitte schöne! =)

Florent