Closed Bug 120756 Opened 21 years ago Closed 21 years ago

function use before <script> declaration considered harmful

Categories

(Bugzilla :: Bugzilla-General, defect, P1)

2.15
x86
Windows 98
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: timeless, Assigned: gerv)

Details

Attachments

(1 file, 1 obsolete file)

IE5.5 and 6 at the very least result in runtime errors if you do it.  The
attachments manager triggers this a lot. a non harmful example is to click edit
attachment as comment. you get a script error and no results.
Much as we all love the lizard, I think that this is a 2.16 blocker. It would
possibly be easier for someone with access to IE to do this one.
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
This doesn't happen with the query.cgi JS-foo, but I agree it's a good thing.
Should this be blocked by the "make JS a separate file" bug to make it easier to
implement?
Attached patch Patch v.1 (obsolete) — Splinter Review
This seems sensible, but I have no ability to test it. I just moved the entire
<script> block to the top of the file.

Gerv
Keywords: patch, review
Need reviews from Javascript folks on this one.
Comment on attachment 67406 [details] [diff] [review]
Patch v.1

r=afranke
Attachment #67406 - Flags: review+
-> patch author
Assignee: justdave → gerv
Comment on attachment 67406 [details] [diff] [review]
Patch v.1

I went to check this in, and I can't get the patch to apply.  diff claims it's
"reversed or already applied", and the patch fails whether I apply it in
reverse mode or not.
Attachment #67406 - Flags: review+ → review-
That patch was diffed against r1.7 of edit.atml, current cvs tip is r1.8.  I did
try applying the patch to r1.7 then updating, but cvs failed to merge it also.
Keywords: patch, review
This is the patch against rev. 1.8 of template/default/attachment/edit.atml .
I have recreated it manually, without using the old patch. (The patch just
swaps the order of occurrence of the main <form> and <script> elements.)
I have tried to test the patch in Netscape4 and Mozilla, and I could not find
any oddities. Seems to work fine.
Attachment #67406 - Attachment is obsolete: true
Comment on attachment 69083 [details] [diff] [review]
Updated patch to rev. 1.8

Re-marking r=afranke.
Attachment #69083 - Flags: review+
Checked in. 

Gerv
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.