597784 - The inserthtml command and nsIHTMLEditor::InsertHTML should not use a sanitizing fragment content sink

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Phillip M. Jones, C.E.T.]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.1.11) Gecko/20100701 Lightning/1.0b1 Mnenhy/0.8.2 SeaMonkey/2.0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.1.12) Gecko/20100701 Lightning/1.0b1 Mnenhy/0.8.2 SeaMonkey/2.0.6

Starting with SM2.0.7 (gecko rv:1.9.1.12) and up when you attempt in html containing items (sound/Video) either using Embed or object tag using the insert HTML in Newsgroups that allow such content, does not appear. You can type the content  and then choose insert. The  insert c ommand appears to be ignored. 

Reproducible: Always

Steps to Reproduce:
1.Begin Compose Message 
2.Go to insert Menu choose HTML
3.type in desired code for inserted content
4. click on insert
Actual Results:  
inserted content does not get inserted
save as Draft to prove Nothing there.

Expected Results:  
In 2.0.6 (rv:1.9.1.11) and Below
inserted content is inserted 
Save as Draft and view Draft and Embed Content works.

You can Try in safe Mode has No affect. Theme doesn't make a difference.  Although I am using Macintosh OSX, I've heard reports from people with other platforms have the same problem.

I while I don't normal send HTML content in newsgroup messages There are some newsgroup That allow it and I use the abilty to test out code before inserting my website or other web pages.

Comment 1

[(no longer active)]

What is the exact HTML code you enter in step 3?

Comment 2

[Phillip M. Jones, C.E.T.]

either:

<EMBED SRC="title" AUTOSTART=TRUE LOOP=FALSE WIDTH=145 HEIGHT=55 ALIGN="CENTER">
</EMBED>


or 

<object
type="application/x-type" data="title"
width="150" height="40" AUTOSTART="FALSE">
<param name="music" value="title" />
</object>


are two samples: 

I use music in these examples  that I have loaded on my website so title would be:

http://www.phillipmjones.net/songtittle/extension

here is  a real example:

<object
type="application/mp3" data="http://www.phillipmjones.net/Brown_Eyed_Handsom_Man.mp3"
width="150" height="40" AUTOSTART="FALSE">
<param name="music" value="http://www.phillipmjones.net/Brown_Eyed_Handsom_Man.mp3" />
</object>

The above code I finally used in my website on a particular page The title was different

or

<EMBED SRC="http://www.phillipmjones.net/Brown_Eyed_Handsom_Man.mp3" AUTOSTART=FALSE LOOP=FALSE WIDTH=145 HEIGHT=55 ALIGN="CENTER">
</EMBED>

 to actually insert title directly hasn't worked since switch from Mozilla to SeaMonkey Note I own this music Title and is not meant to be copied. 

Note I may have typed wrong in live examples but the original code came from the internet and is accurate.

Comment 3

[Phillip M. Jones, C.E.T.]

<object>
<param name="autostart" value="true">
<param name="src" value="eureka.wav">
<param name="autoplay" value="true">
<param name="controller" value="true">
<embed src="eureka.wav" controller="true" autoplay="true" autostart="True" type="audio/wav" />
</object>
 this a another version

in live example would be :

<object>
<param name="autostart" value="false">
<param name="src" value="http://www.phillipmjones.net/Brown_Eyed_Handsom_Man.mp3">
<param name="autoplay" value="false">
<param name="controller" value="true">
<embed src="http://www.phillipmjones.net/Brown_Eyed_Handsom_Man.mp3" controller="true" autoplay="false" autostart="False" type="audio/mp3" />
</object>

Comment 4

[Phillip M. Jones, C.E.T.]

the second example just above is what I now use on my website.

Comment 5

[(no longer active)]

Morphing this bug to switch the inserthtml command not to use a sanitizing fragment content sink.

Comment 7

[Phillip M. Jones, C.E.T.]

I use the ability to to test such code in newsgroups that allow such (Embed and Object)  so that I can get a second opinion on something I can use in my website. 

In fact since this bug has reared its ugly head. I have discontinued upgrading SeaMonkey at 2.0.6 Gecko  rv:1.9.1.11. And I will continue not upgrading until it is fixed or some method is created to bypass it through about config, a menu choice, or someone comes up a hack to get around it. 

No one has the right to tell me how I can use an application (especially an open Source application that depends upon the good graces of users to promote it). Most of the people  that use such are honest, and intend no harm.

Instead of going about the way why not have a preference setting to allow person on other end to block the items. Then the users get to choose whether they want to do or see or heard such. 

Again we have developer run amuck, deciding what they feel is right for the user rather than letting the users decide.

Comment 8

[(no longer active)]

Attachment: Patch (v1)

Comment 12

[hans]

Will the paste command be modified as well?

Comment 13

[(no longer active)]

(In reply to comment #12)
> Will the paste command be modified as well?

No.

Comment 15

[hans]

(In reply to comment #13)
> (In reply to comment #12)
> > Will the paste command be modified as well?
> 
> No.

So when this patch is applied, a user (of Seasmonkey / Thunderbird composer or some CMS in FF), will be able to insert an object, but won't be able to move it using cut/paste. That's not very intuitive.

Comment 16

[(no longer active)]

(In reply to comment #15)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > Will the paste command be modified as well?
> > 
> > No.
> 
> So when this patch is applied, a user (of Seasmonkey / Thunderbird composer or
> some CMS in FF), will be able to insert an object, but won't be able to move it
> using cut/paste. That's not very intuitive.

SeaMonkey and Thunderbird can switch to using the InsertHTML API if they want to enable pasting unfiltered HTML.

Comment 17

[hans]

Ok. Thanks for updating the white list of the sanitizing fragment sink. It would be nice to have the data-* attributes white listed too. Otherwise web developers may start abusing other attributes or elements to fit their needs and workaround the behavior in Firefox.
http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data

Comment 18

[(no longer active)]

(In reply to comment #17)
> Ok. Thanks for updating the white list of the sanitizing fragment sink. It
> would be nice to have the data-* attributes white listed too. Otherwise web
> developers may start abusing other attributes or elements to fit their needs
> and workaround the behavior in Firefox.
> http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#embedding-custom-non-visible-data

See bug 598105.

Comment 19

[Daniel Veditz [:dveditz]]

We need to make sure this fix doesn't regress the security problem we were originally addressing.

Comment 20

[(no longer active)]

(In reply to comment #19)
> We need to make sure this fix doesn't regress the security problem we were
> originally addressing.

It won't.  These are two separate code paths, and we have tests for the security fix.

Comment 21

[(no longer active)]

bz: review ping...

Comment 22

[Osbron]

After fixing this bug, I would recommend this patch to be included into the newer version of fiefox 3.6.11 so that everyone would be able to enjo this patch. As seen from the responses, this problem had affected many, and thus its highly recommended that this bug is fixed, and fast.

Other users experiencing this problem are mainly website developers especially using joomla.

Comment 23

[(no longer active)]

Osbron: the fix to this bug will be shipped in the text dot release of Firefox 3.5 and 3.6.

Comment 24

[Phillip M. Jones, C.E.T.]

will this also be fixed in next "." version of SeaMonkey as well?

Comment 25

[(no longer active)]

(In reply to comment #24)
> will this also be fixed in next "." version of SeaMonkey as well?

Kairo is probably a better person to answer this question.  I don't know a lot about SM's release schedule.

Comment 26

[Robert Kaiser]

Anything that goes into the platform for the next Firefox 3.5 release goes into the next SeaMonkey 2.0 release, so given comment #23, the answer seems to be yes.

Comment 27

[(no longer active)]

Attachment: Patch (v1)

I just realized that I had forgotten to add a test that I wrote to the patch!

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 478369 [details] [diff] [review]
Patch (v1)

You don't need approval; this is a blocker.

I'd prefer DoInsertHTMLWithContext as the function name.  The header needs some documentation.  And the copy/pasted comments in the code you moved have typos.  r=me with all that fixed.

Comment 29

[Boris Zbarsky [:bzbarsky]]

I do wonder whether this needs to block b7 or not, though...

Comment 31

[(no longer active)]

(In reply to comment #29)
> I do wonder whether this needs to block b7 or not, though...

Well, I'm going to land it today since it's a branch blocker as well, but yes, I'd think that this should have been a beta7 blocker...

Comment 32

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/b9ccfd3d502a

Comment 33

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4e58ef70f2eb
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a537a1541695

Comment 34

[(no longer active)]

Test fix follow-up on branches: nsIEditor::PasteFromTransferable is not available on branches, so I've disabled the part of the test which used it.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/22b0a1db44b6
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/69d435c2f4ec

Comment 35

[Wolfgang Rosenauer [:wolfiR]]

Disabling the part has bad side effects? Please see orange.

Comment 36

[Mark Banner (:standard8)]

Backed out from 1.9.2 due to persisting test failures:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/95a205a82cb8
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ef28900b1c02

Please check 1.9.1 as well.

Comment 37

[(no longer active)]

(In reply to comment #36)
> Backed out from 1.9.2 due to persisting test failures:
> 
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/95a205a82cb8
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ef28900b1c02
> 
> Please check 1.9.1 as well.

Oh, sorry about the oranges.  :(  Backporting tests to branches is harder than I expected.  I'll reland with a test fix later today.

Comment 38

[(no longer active)]

Relanded with test fix:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/66b8048d9b02
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0785fb638958

Comment 39

[(no longer active)]

Test fix ported to 1.9.1:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0ee5091e706e

Comment 43

[Graham]

Frank,
Repeat of Bug 601483: Just tested SM 2.0.9 and 2.0.10 - NOT resolved, NOT fixed: the fault is still the same. 
In SeaMonkey Composer 2.0.9 and 2.0.10, copy-and-paste of onclick='document.location.href="#top"' fails. Version 2.0.6 is OK.
Graham.

Comment 44

[(no longer active)]

(In reply to comment #43)
> Frank,
> Repeat of Bug 601483: Just tested SM 2.0.9 and 2.0.10 - NOT resolved, NOT
> fixed: the fault is still the same. 
> In SeaMonkey Composer 2.0.9 and 2.0.10, copy-and-paste of
> onclick='document.location.href="#top"' fails. Version 2.0.6 is OK.
> Graham.

Please file a bug in the appropriate SeaMonkey component for this.

Comment 45

[Frank Wein [:mcsmurf]]

Bug 601483 has been filed on this, I marked it as a dupe of this one a while ago as I thought this bug should have fixed the bug. Is inserting
<div style="text-align: right;"> <input value="Back to Top"
onclick='document.location.href="#top"' type="button"> </div>
via the Insert HTML... dialog another problem?

Comment 46

[(no longer active)]

(In reply to comment #45)
> Bug 601483 has been filed on this, I marked it as a dupe of this one a while
> ago as I thought this bug should have fixed the bug. Is inserting
> <div style="text-align: right;"> <input value="Back to Top"
> onclick='document.location.href="#top"' type="button"> </div>
> via the Insert HTML... dialog another problem?

Depends on how SeaMonkey has implemented its Insert HTML command.  If it's using nsIHTMLEditor::InsertHTML, then this should be fixed in this bug.  Otherwise, it should switch to using that API!  :-)

Comment 47

[Jérôme Poirrier]

(In reply to comment #43)
> Frank,
> Repeat of Bug 601483: Just tested SM 2.0.9 and 2.0.10 - NOT resolved, NOT
> fixed: the fault is still the same. 
> In SeaMonkey Composer 2.0.9 and 2.0.10, copy-and-paste of
> onclick='document.location.href="#top"' fails. Version 2.0.6 is OK.
> Graham.

November 28th 2010
I've got the same thing, with SM 2.0.9 and 2.0.10. When I copy-paste a button, the javascript properties are lost in the resulting copies. 

I have felt complied to revert to SM 2.0.2 several times (unfortunately, as a kee-jerk reaction,I sometimes do allow automatic upgrading  SM 2.0.10, and I can no longer work and it drives me crazy). 

How on earth can this bug be deemed "RESOLVED FIXED" ??!!!

Comment 48

[Jérôme Poirrier]

Oops, sorry for the typo : read "knee-jerk reaction" instead of "kee-jerk"

Comment 49

[Jérôme Poirrier]

Oops, another typo :   upgrading *to* SM 2.0.10

Comment 50

[Jérôme Poirrier]

Oops III : "compelled" instead of "complied".
(It's time I got myself some sleep instead of coffee, 
and I'd better post the whole msg again) :

November 28th 2010
I've got the same thing, with SM 2.0.9 and 2.0.10. When I copy-paste a button,
the javascript properties are lost in the resulting copies. 

I have felt compelled to revert to SM 2.0.2 several times (unfortunately, as a
knee-jerk reaction,I sometimes do allow automatic upgrading to SM 2.0.10, and I
can no longer work and it drives me crazy). 

How on earth can this bug be deemed "RESOLVED FIXED" ??!!!

Comment 51

[(no longer active)]

Please file a new bug in the SeaMonkey specific component and ask the SeaMonkey developers to pick up the fix to this bug.  This has been fixed in core, and I'm not sure if SeaMonkey has plans to pick up the fix for this bug, but that doesn't have anything to do with this core bug.

Comment 52

[Graham]

Response to comment 51 (comments 43 from me and 50 from Jerome refer):
Ehsan, this issue is being bounced around with no-one seeming to take ownership. I originated bug report 601483 in SeaMonkey Composer - is that the 'specific component' you mention?
Somebody thought it was a dupe of this 597784. Whoever it belongs to, it is NOT fixed. I will post this same message in 601483 and hope someone picks it up.
Thanks.
Graham.

Comment 53

[(no longer active)]

Please stop posting SeaMonkey specific stuff here.  This is a core bug.  For issues with SeaMonkey, you need to either talk to SM developers or file bugs in SM bugzilla components.