Bug 7640

Summary: Menu items are displayed as "undefined"
Product: WebKit Reporter: Rudolf E. Reiber <rudolfreiber>
Component: DOMAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, darin, ian, sam
Priority: P2 Keywords: HasReduction
Version: 417.x   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Screenshot Firefox
none
Screenshot Safari
none
The source code of the page
none
proposed fix
darin: review+
proposed fix darin: review+

Description Rudolf E. Reiber 2006-03-07 01:28:46 PST
I have to use a forum in the BSCW-platform. It is a platform open only to members, so I cannot give a URL to it.
When I use the BSCW with Firefox, all pop-up-menues are displayed properly, when using Safari, all menue-items are displayed as "undefined".
Comment 1 Rudolf E. Reiber 2006-03-07 01:33:21 PST
Created attachment 6912 [details]
Screenshot Firefox
Comment 2 Rudolf E. Reiber 2006-03-07 01:34:26 PST
Created attachment 6913 [details]
Screenshot Safari
Comment 3 Gustaaf Groenendaal (MysteryQuest) 2006-03-07 02:18:56 PST
Could you please provide us with the source code of the page. The best way is opening the page with FireFox and choose "Save As". Upload this as a .zip archive in an attachment. If there is private information on the page you can filter that out ofcourse.
Comment 4 Rudolf E. Reiber 2006-03-07 02:35:03 PST
Created attachment 6914 [details]
The source code of the page
Comment 5 Alexey Proskuryakov 2007-01-07 07:43:43 PST
Created attachment 12277 [details]
proposed fix

The problem here was that a local variable "item" got shadowed by Node::item within a "with" block. AFAICT, there is no reason to have this method on Node. "Gecko DOM reference" cited in code comment is probably <http://developer.mozilla.org/en/docs/DOM:element.item>, which specifically states that "despite this article's name, item() is not a method of DOM Element or Node."

The included test passes in Firefox 2 and IE 6.
Comment 6 Darin Adler 2007-01-07 21:25:56 PST
Comment on attachment 12277 [details]
proposed fix

Looks good.

Would you please add the item function to HTMLSelectElement.idl/h/cpp instead of adding it to kjs_html.h/cpp?
Comment 7 Alexey Proskuryakov 2007-01-08 01:28:56 PST
(In reply to comment #6)
> Would you please add the item function to HTMLSelectElement.idl/h/cpp instead
> of adding it to kjs_html.h/cpp?

HTMLSelectElement JS bindings are not generated from IDL (yet?), so I guess this code has to go to kjs_html. I'm not sure if we need this method in ObjC binding, its standards status seems somewhat unclear.
Comment 8 Darin Adler 2007-01-08 10:09:39 PST
(In reply to comment #7)
> HTMLSelectElement JS bindings are not generated from IDL (yet?), so I guess
> this code has to go to kjs_html.

OK. Argh. Well, I'd still like HTMLSelectElement.h/cpp to have the code, even if the binding is old-style. I marked the patch review+, though.

> I'm not sure if we need this method in ObjC binding, its standards status seems somewhat unclear.

Putting methods in ObjC at one time was based on what was in the W3C standard, but nowadays we'd like the ObjC binding to be as close as possible to what's available through JavaScript. And the system automatically keeps new calls SPI, so there's no problem with Apple API policy.
Comment 9 Alexey Proskuryakov 2007-01-08 11:26:13 PST
Created attachment 12302 [details]
proposed fix

Added item() to DOM and ObjC binding, too.

Not sure if it should be marked const in DOM. It returns a non-const pointer to object data, so it probably shouldn't, but is marked const in NodeList, for example.
Comment 10 Darin Adler 2007-01-08 11:28:42 PST
Comment on attachment 12302 [details]
proposed fix

r=me
Comment 11 Alexey Proskuryakov 2007-01-08 12:00:22 PST
Committed revision 18674.