Closed Bug 122301 Opened 19 years ago Closed 12 years ago

"edit attachment as comment" abuses xmlserialiser

Categories

(Bugzilla :: Attachments & Requests, defect, P2)

2.15
defect

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: bbaetz, Assigned: LpSolit)

References

()

Details

(Whiteboard: [blocker 38862 will fix])

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.
Blocks: 86012
OS: Linux → All
Hardware: PC → All
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.
Depends on: 102957
Blocks: 126273
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
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 nobody@bugzilla.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 → ---
QA Contact: mattyt-bugzilla → default-qa
Assignee: myk → attach-and-request
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
No longer blocks: 86012
You need to log in before you can comment on or make changes to this bug.