Closed
Bug 597784
Opened 13 years ago
Closed 13 years ago
The inserthtml command and nsIHTMLEditor::InsertHTML should not use a sanitizing fragment content sink
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
People
(Reporter: pjonescet, Assigned: ehsan.akhgari)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
22.18 KB,
patch
|
bzbarsky
:
review+
christian
:
approval1.9.2.11+
christian
:
approval1.9.1.14+
|
Details | Diff | Splinter Review |
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.
Updated•13 years ago
|
Keywords: regression,
regressionwindow-wanted
Version: unspecified → SeaMonkey 2.0 Branch
![]() |
||
Updated•13 years ago
|
Component: General → Composition
Product: SeaMonkey → MailNews Core
QA Contact: general → composition
Version: SeaMonkey 2.0 Branch → unspecified
Assignee | ||
Comment 1•13 years ago
|
||
What is the exact HTML code you enter in step 3?
Reporter | ||
Comment 2•13 years ago
|
||
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.
Reporter | ||
Comment 3•13 years ago
|
||
<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>
Reporter | ||
Comment 4•13 years ago
|
||
the second example just above is what I now use on my website.
Assignee | ||
Updated•13 years ago
|
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ehsan
Component: Composition → Editor
Product: MailNews Core → Core
QA Contact: composition → editor
Version: unspecified → Trunk
Assignee | ||
Updated•13 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Assignee | ||
Comment 5•13 years ago
|
||
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
Reporter | ||
Comment 7•13 years ago
|
||
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.
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #477032 -
Flags: review?(bzbarsky)
Attachment #477032 -
Flags: approval2.0?
Assignee | ||
Updated•13 years ago
|
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
Comment 12•13 years ago
|
||
Will the paste command be modified as well?
Assignee | ||
Comment 13•13 years ago
|
||
(In reply to comment #12) > Will the paste command be modified as well? No.
Comment 15•13 years ago
|
||
(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.
Assignee | ||
Comment 16•13 years ago
|
||
(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•13 years ago
|
||
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
Assignee | ||
Comment 18•13 years ago
|
||
(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•13 years ago
|
||
We need to make sure this fix doesn't regress the security problem we were originally addressing.
Assignee | ||
Comment 20•13 years ago
|
||
(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.
Assignee | ||
Comment 21•13 years ago
|
||
bz: review ping...
Comment 22•13 years ago
|
||
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.
Assignee | ||
Comment 23•13 years ago
|
||
Osbron: the fix to this bug will be shipped in the text dot release of Firefox 3.5 and 3.6.
Reporter | ||
Comment 24•13 years ago
|
||
will this also be fixed in next "." version of SeaMonkey as well?
blocking2.0: ? → betaN+
Assignee | ||
Comment 25•13 years ago
|
||
(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•13 years ago
|
||
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.
Assignee | ||
Comment 27•13 years ago
|
||
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 28•13 years ago
|
||
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?
![]() |
||
Comment 29•13 years ago
|
||
I do wonder whether this needs to block b7 or not, though...
Assignee | ||
Comment 31•13 years ago
|
||
(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...
Assignee | ||
Updated•13 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 32•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b9ccfd3d502a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b7
Assignee | ||
Updated•13 years ago
|
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+
Assignee | ||
Comment 33•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4e58ef70f2eb http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a537a1541695
status1.9.1:
--- → .14-fixed
status1.9.2:
--- → .11-fixed
Assignee | ||
Comment 34•13 years ago
|
||
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•13 years ago
|
||
Disabling the part has bad side effects? Please see orange.
Comment 36•13 years ago
|
||
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.
status1.9.2:
.11-fixed → ---
Assignee | ||
Comment 37•13 years ago
|
||
(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.
Assignee | ||
Comment 38•13 years ago
|
||
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
status1.9.2:
--- → .11-fixed
Assignee | ||
Comment 39•13 years ago
|
||
Test fix ported to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0ee5091e706e
Comment 43•13 years ago
|
||
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.
Assignee | ||
Comment 44•13 years ago
|
||
(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•13 years ago
|
||
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?
Assignee | ||
Comment 46•13 years ago
|
||
(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•13 years ago
|
||
(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•13 years ago
|
||
Oops, sorry for the typo : read "knee-jerk reaction" instead of "kee-jerk"
Comment 49•13 years ago
|
||
Oops, another typo : upgrading *to* SM 2.0.10
Comment 50•13 years ago
|
||
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" ??!!!
Assignee | ||
Comment 51•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 53•13 years ago
|
||
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.
Description
•