Closed Bug 597784 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: DOM: Editor, defect)

PowerPC
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- betaN+
blocking1.9.2 --- .11+
status1.9.2 --- .11-fixed
blocking1.9.1 --- .14+
status1.9.1 --- .14-fixed

People

(Reporter: pjonescet, Assigned: ehsan)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
Version: unspecified → SeaMonkey 2.0 Branch
Component: General → Composition
Product: SeaMonkey → MailNews Core
QA Contact: general → composition
Version: SeaMonkey 2.0 Branch → unspecified
What is the exact HTML code you enter in step 3?
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.
<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>
the second example just above is what I now use on my website.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → ehsan
Component: Composition → Editor
Product: MailNews Core → Core
QA Contact: composition → editor
Version: unspecified → Trunk
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Morphing this bug to switch the inserthtml command not to use a sanitizing fragment content sink.
Status: NEW → ASSIGNED
Summary: HTML Content such as embedded sound/Video or Object tag in Newsgroups no longer works → The inserthtml command should not use a sanitizing fragment content sink
Duplicate of this bug: 580442
Blocks: 596300
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.
Attached patch Patch (v1) (obsolete) — Splinter Review
Attachment #477032 - Flags: review?(bzbarsky)
Attachment #477032 - Flags: approval2.0?
Duplicate of this bug: 597393
Duplicate of this bug: 598287
Duplicate of this bug: 595176
Summary: The inserthtml command should not use a sanitizing fragment content sink → The inserthtml command and nsIHTMLEditor::InsertHTML should not use a sanitizing fragment content sink
Will the paste command be modified as well?
(In reply to comment #12)
> Will the paste command be modified as well?

No.
Duplicate of this bug: 596513
(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.
(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.
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
(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.
blocking1.9.1: ? → .14+
blocking1.9.2: ? → .11+
We need to make sure this fix doesn't regress the security problem we were originally addressing.
(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.
bz: review ping...
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.
Osbron: the fix to this bug will be shipped in the text dot release of Firefox 3.5 and 3.6.
will this also be fixed in next "." version of SeaMonkey as well?
(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.
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.
Attached patch Patch (v1)Splinter Review
I just realized that I had forgotten to add a test that I wrote to the patch!
Attachment #477032 - Attachment is obsolete: true
Attachment #478369 - Flags: review?(bzbarsky)
Attachment #478369 - Flags: approval2.0?
Attachment #477032 - Flags: review?(bzbarsky)
Attachment #477032 - Flags: approval2.0?
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.
Attachment #478369 - Flags: review?(bzbarsky)
Attachment #478369 - Flags: review+
Attachment #478369 - Flags: approval2.0?
I do wonder whether this needs to block b7 or not, though...
Duplicate of this bug: 599316
(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...
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/b9ccfd3d502a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b7
Attachment #478369 - Flags: approval1.9.2.11?
Attachment #478369 - Flags: approval1.9.1.14?
Attachment #478369 - Flags: approval1.9.2.11?
Attachment #478369 - Flags: approval1.9.2.11+
Attachment #478369 - Flags: approval1.9.1.14?
Attachment #478369 - Flags: approval1.9.1.14+
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
Disabling the part has bad side effects? Please see orange.
(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.
Duplicate of this bug: 601483
Duplicate of this bug: 597190
Duplicate of this bug: 603219
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.
(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.
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?
(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!  :-)
(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" ??!!!
Oops, sorry for the typo : read "knee-jerk reaction" instead of "kee-jerk"
Oops, another typo :   upgrading *to* SM 2.0.10
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" ??!!!
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.