Closed Bug 184509 Opened 23 years ago Closed 22 years ago

midas - unable to insert html (cmd_insertHTML)

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.5alpha

People

(Reporter: Brade, Assigned: Brade)

Details

(Whiteboard: midas)

Attachments

(2 files, 1 obsolete file)

testcase
400 bytes, text/html
Details
patch that works
2.51 KB, patch
mkaply
: review+
kinmoz
: superreview+
mkaply
: approval1.4+
Details | Diff | Splinter Review
Midas needs to be able to insert a table
Status: NEW → ASSIGNED
Whiteboard: midas
Here's an example of how this works in IE: var cursorPos = this.opener.docView.document.selection.createRange().duplicate(); cursorPos.pasteHTML( tbl ); So what's missing is the ability to do an arbitrary paste of HTML.
I'm not sure how we want to do this. I understand wanting to insert a table but we can't allow arbitrary html to be inserted since I'm pretty sure there would be exploits (e.g. iframe with src=file:///quickenfile.txt) What tags can we not allow to be inserted? is IFRAME the only problematic tag?
Summary: midas - unable to insert a table → midas - unable to insert html (cmd_insertHTML)
I'm sorry, I still don't understand the exploit scenario. Just inserting HTML into the iframe wouldn't cause anything to happen...
Sure it would, if the HTML that's inserted contains a <SCRIPT> tag, or an <IFRAME SRC="javascript:DoSomethingAnnoying()">. The issue here is the same-origin sandbox. It's OK for a script from site A to add arbitrary HTML to a page that was also served from site A, but not to a page from site B (eg. your bank site). AFAIK, that's already the case, so we shouldn't have any problem here, right Kathy? This isn't a paste from the Clipboard, is it? We don't want to allow that.
It is already possible insert any html using dom, and having a simple way to do so at the current location of the cursor would be very usefull. Example: The current way to make a link has you type in text, then highlight it and then you get a popup for the url. It is much more commen and user friendly (imho) to have both the text and url in the popup and the link added to the location of the cursor. A inserHTML type command would make this much easier to implement.
Mitch - so do you see anything insecure with allowing this? I could easily add a new script element using pure DOM without this command available - it just makes it easier for people to add their own "semantic" tags into their composition.
Sounds OK as long as only a script from the same host as the page being edited can insert HTML - please verify that.
Can someone post a patch to turn on html insertion via midas so I can test mstoltz's concerns?
should this go into 1.4 final?
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-
Attached patch preliminary patch without checks (obsolete) — — Splinter Review
Attached file testcase —
Testcase
Attached patch patch that works — — Splinter Review
This patch works but doesn't check the content being inserted for bad things (not sure if they're really needed); will test more tomorrow
Attachment #124484 - Attachment is obsolete: true
Attachment #124506 - Flags: superreview?(sfraser)
Attachment #124506 - Flags: review?(mkaply)
Comment on attachment 124506 [details] [diff] [review] patch that works sr=kin@netscape.com ==== We're not currently sending cstrings, so does this comment still apply? + // for inserthtml and fontface, we should send utf8 strings not cstrings ==== The checks would probably be made in editor core, so I wouldn't put this comment in nsDocument.cpp: + // do some checks here for <frame>, <body>, <html>, others?
Attachment #124506 - Flags: superreview?(sfraser) → superreview+
Comment on attachment 124506 [details] [diff] [review] patch that works r=mkaply
Attachment #124506 - Flags: review?(mkaply) → review+
Comment on attachment 124506 [details] [diff] [review] patch that works seeking 1.4 approval after this has baked on the trunk.
Attachment #124506 - Flags: approval1.4?
fixed for 1.5a Documentation also updated. (mozilla/editor/docs/midas-spec.html)
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.4final
Comment on attachment 124506 [details] [diff] [review] patch that works a=mkaply for 1.4
Attachment #124506 - Flags: approval1.4? → approval1.4+
did this ever land in 1.4?
Doron--I believe mkaply was going to land this for 1.4 but from what I see it did not land in the 1.4 tree (yet). It landed as revision 3.487 to nsHTMLDocument.cpp on the trunk. Resetting milestone to avoid confusion. If mkaply or someone else fixes for 1.4 branch, we can reset milestone to 1.4final again.
Target Milestone: mozilla1.4final → mozilla1.5alpha
I didn't realize this one was mine to checkin. I just approved it. Dang.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: