Closed Bug 122301 Opened 19 years ago Closed 12 years ago
"edit attachment as comment" abuses xmlserialiser
I filed bug 121776, about edit attachment as comment truncating stuff. This was a WFM, but heikki mnetioned that we are abusing xmlserialiser by serialising non-xml documents. The suggestion was to use XMLHttpRequest to grab the attachment, and then we can just insert it twice into the <iframe>, formatted as appropriate, using .responseText to get the actual response. In order to support browsers with no js, the existing code would be in a <noscript> tag. The edit attachment button shouldn't be displayed in that case, anyway, since it can't work w/o js. (Well, it could with a round trip to the server, but thats a separate issue)
Once this is fixed, I am going fix 86012 so that it will throw an error if you try to serialize non-XML document.
OS: Linux → All
Hardware: PC → All
19 years ago
The problem with doing it this way is that there is no way to set the content-type on the iframe so that the data is displayed correctly. Even if I don't use an iframe, I don/t know if theres a way to do it Actually, maybe document.open has a type argument, or I could use a data url, or something. Is that portable?
document.load() also loads XML, so it won't do. I guess I don't understand what the problem you are describing is. What I would expect is something like this: 1. Page loaded with JS. 2. XMLHttpRequest starts loading attachment. 3. When readyState is LOADED, you can see the attachment's mime type with getResponseHeader(). 3.1 If it is not text attachment, you abort(), create iframe and set src to the attachment URL. 3.2 If it is text, you create iframe and put responseText there once loaded (or dynamically update it if you want for each INTERACTIVE readyState). 4. Create "Edit" button. In case JS is disabled we'd put iframe with src there in a noscript section. About step 4.... You could maybe have the button there earlier, disabled, reading "Loading..." or something and then changed the text to "Edit" and enable it once it has loaded. Currently, if you hit the "Edit" button before the attachment is loaded it will submit the page!
Well, we know the mimetype before we start, so we can avoid the abort(). The question really is: Do we want to support "edit attachment as comment" for non text/plain type? If we don't, then this will work. If we do, then we have to convince the browser to display the responeText as html somehow For step 3.2 + text/plain, we can just append text nodes to the contentDocument, right?
If the attachment was HTML, you could perhaps document.write responseText. But you'd still be in trouble if it was XUL or image or anything else. I'd say enable edit only for text/plain (patches mostly) for now, and worry about editing other types later.
Isn't this all horribly Mozilla-specific anyway? Don't we have some other way to do this stuff?
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
Theres a DOM3 draft to do this, but I don't think its at the implementable stage yet. Since we require scripting anyway, there may be something we can do with data urls, since I assumne all browsers which support js support that.
Right the scripting is just required for the IFRAME though isn't it?
Well, yes, but the iframe is the only part which is affected by this. Acctually, if we only edit text/plain then we can just use document.src. The problem with that is that then we either send two copies, or require scripting in teh first iframe. Maybe we could serialise the inital iframe src, though. I'll have to play with it a bit, but this won't happen for 2.16
per bbaetz's comment #9, "this won't happen for 2.16"
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
19 years ago
Component: Creating/Changing Bugs → attachment and request management
Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being retargeted to 2.20 If you plan to act on one immediately, go ahead and pull it back to 2.18.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
Really, we could also just get the text during a CGI by making "Edit Attachment as Comment" reload the page. Either way, yeah, we can just assume that it's all text/plain. Right now, I hate what the xmlserializer does to my HTML tags.
This bug has not been touched by its owner in over six months, even though it is targeted to 2.20, for which the freeze is 10 days away. Unsetting the target milestone, on the assumption that nobody is actually working on it or has any plans to soon. If you are the owner, and you plan to work on the bug, please give it a real target milestone. If you are the owner, and you do *not* plan to work on it, please reassign it to firstname.lastname@example.org or a .bugs component owner. If you are *anybody*, and you get this comment, and *you* plan to work on the bug, please reassign it to yourself if you have the ability.
Target Milestone: Bugzilla 2.20 → ---
We will no longer use XMLSerializer once bug 38862 is fixed.
Assignee: attach-and-request → LpSolit
Depends on: 38862
Whiteboard: [blocker 38862 will fix]
Target Milestone: --- → Bugzilla 3.0
Bug 38862 has been checked in -> FIXED!
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.