108547 - javascript error in attachment.cgi - using bad DOM Style code

Status: RESOLVED FIXED

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

Description

[timeless]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.5+) Gecko/20011105

Error: small.style.getProperty is not a function
Source File: http://bugzilla.mozilla.org/attachment.cgi?id=56564&action=edit
Line: 212

this build is generally borked, i'm not sure what caused this. oh wait, sure i
am :-)

feel free to dupe.

Comment 1

[timeless]

it's all jkeiser's fault!

Comment 2

[John Keiser (jkeiser)]

Um.  Are new bugs just getting assigned to me by default now or something? :)

First, that attachment doesn't even have JavaScript in it--wrong attachment #?

Second, my patch modified a couple of GetProperty methods in *frames*, but I
didn't touch style in the least.  And form control content doesn't have any
getProperty() method to begin with ...

I really have no idea who this goes to though, esp. since I can't see the
testcase :)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Okeee...  the page in question has:

 var small = document.getElementById('smallCommentFrame');
 if ( small.style.getProperty('display') == 'none' )

Too bad that .style gives you a CSSStyleDeclaration and that has no
getProperty() method, just a getPropertyValue().  Over to bugzilla -- this is a
bug in the bugzilla JS pure and simple.

Comment 4

[Dave Miller [:justdave]]

CCing the javascript experts on the Bugzilla team.

I do know the page in question has been completely redone in CVS since the last
time bugzilla.mozilla.org updated, but I don't know if that affected the javascript.

Comment 5

[John Keiser (jkeiser)]

Oh oh oh!  Sorry timeless, I didn't realize I was supposed to be looking at the
actual attachment.cgi page, I was looking at the patch :)

Comment 6

[John Keiser (jkeiser)]

Well, that rewrite may have affected the JS but this error does not occur on
Linux 2001103012 ... so it may be a combo of the two.

Comment 7

[Dave Miller [:justdave]]

what I meant by the page being redone is that the javascript in question may not
even be there anymore on the current version (bugzilla.mozilla.org is not
running the current version until this coming Wednesday night)

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  

STEPS TO REPRODUCE:

1) Load an attachment page
2) Submit the form

The onsubmit() handler should fire and cause those errors.  If it does not, it's
a  bug.  If it did not fire prior to jkeiser's checkin.... well... :)

jkeiser, did you actually submit the form when testing?

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking.

Comment 10

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch to fix

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

The attached patch gets rid of the bad style.(get|set)Property(foo) and replaces
it with style.foo code plus a minor cleanup.

Comment 12

[timeless]

Comment on attachment 56606 [details] [diff] [review]
Patch to fix

>   function hideElementById(id)
>   {
>     var elm = document.getElementById(id);
>-    if (!elm) return;
>-
>-    elm.style.setProperty('display', 'none', '');
>+    if (elm) {
>+      elm.style.display = 'none';
>+    }
remove this:
>+    return;
>   }

>   function showElementById(id, val)
>   {
>     var elm = document.getElementById(id);
>-    if (!elm) return;
>-
>-    if (!val) val = 'inline';
>-    elm.style.setProperty('display', val, '');
>+    if (elm) {
>+      if (!val) val = 'inline';
>+      elm.style.display = val;
>+    }
remove this:
>+    return;
>   }

r=timeless w/ those removed

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v2

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 56689 [details] [diff] [review]
Patch v2

This patch removes the two returns and timeless said:

r=timeless w/ those removed

Comment 15

[Christian Reis]

Comment on attachment 56689 [details] [diff] [review]
Patch v2

r=kiko good work c.

Comment 16

[Jacob Steenhagen]

Checking in template/default/attachment/edit.atml;
/cvsroot/mozilla/webtools/bugzilla/template/default/attachment/edit.atml,v  <--
 edit.atml
new revision: 1.5; previous revision: 1.4
done