Closed Bug 310927 Opened 19 years ago Closed 19 years ago

Javascript Options.add() is broken in 1.6a1

Categories

(Core :: DOM: Core & HTML, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.8rc1

People

(Reporter: mozilla, Assigned: mrbkap)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051003 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051003 Firefox/1.6a1 Adding an OPTION object to the list of options using the add() method/function is broken in 1.6a1 - 1.6a1 complains that "select.options.add is not a function", yet select.options.add() works fine in IE and previous FF 1.5 beta's such as: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 Test case to follow. Reproducible: Always Steps to Reproduce: 1. Dynamically add an option object using <selectObj>.options.add(newOption) 2. 3. Actual Results: Error: <selectObj>.options.add is not a function Expected Results: New option should have been appended to the list of options - this is valid code and works in IE and previous version of FF.
This regressed between 2005-09-24 and 2005-09-25, and that makes the fix for bug 27382 the most logical candidate responsible for the regression.
Blocks: 27382
Status: UNCONFIRMED → NEW
Component: General → DOM: Core
Ever confirmed: true
Keywords: regression, testcase
Product: Firefox → Core
Version: unspecified → Trunk
(In reply to comment #2) > This regressed between 2005-09-24 and 2005-09-25, and that makes the fix for bug > 27382 the most logical candidate responsible for the regression. Er, bug 27382 really isn't a logical candidate to have caused this. How about bug 296230?
Ah, ok, sorry.
Assignee: nobody → general
Blocks: 296230
No longer blocks: 27382
QA Contact: general → ian
I don't mean to badger, but is this likely to be fixed before release - this bug will ensure we cannot support FF as our site depends on the options.add() functionality! :(
(In reply to comment #5) > I don't mean to badger, but is this likely to be fixed before release - this bug > will ensure we cannot support FF as our site depends on the options.add() > functionality! :( Sure, there's still a lot of time before the 1.6 release. Or are you saying this bug happens in 1.5?
Hi Peter 1.6a1 only I believe - there is no problem with "Firefox Beta 1" which has the following user-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 I think that is 1.5b1? Sorry for any confusion over version numbers and releases, I guess there is no great rush for this fix as it's not in 1.5b1, but it'll be murder if it's forgotten! ;)
|select.options[select.options.length] = newOption| doesn't work either (Exception... "Unexpected error"); the good old W3C methods select.appendChild() and select.add() are not affected.
Assignee: general → peterv
Stealing this to fix for RC1. The patch in bug 296230 is causing GetProperty to stomp over the Add function. The fix is not to stomp over anything, and simply use JSPROP_SHARED.
Assignee: peterv → mrbkap
Flags: blocking1.8rc1?
Status: NEW → ASSIGNED
Attached patch Like so (deleted) — Splinter Review
This uses JSPROP_SHARED to make sure the JS engine doesn't store the property in a slot.
Attachment #198676 - Flags: superreview?(brendan)
Attachment #198676 - Flags: review?(jst)
Bleh, I was about to attach that.
BTW, the patch for bug 296230 was never landed on the branch, so this is not a problem on the branch.
Comment on attachment 198676 [details] [diff] [review] Like so sr=me. /be
Attachment #198676 - Flags: superreview?(brendan) → superreview+
After talking with jst, we decided the additional fix for the branch is not worth pushing into RC1 (given that the bug has been around since 2003), so I'm removing my blocking nomination.
Flags: blocking1.8rc1?
Comment on attachment 198676 [details] [diff] [review] Like so Yeah, duh... r=jst
Attachment #198676 - Flags: review?(jst) → review+
I don't have a problem fixing this for 1.8 if we get it into the trunk and bake it a bit. But then the wrongness of setting *vp unconditionally stands out to me, and the safety of JSPROP_SHARED here is clear. It may not seem that way to everyone, and our regression rate isn't all that hot. But if we bake well on the trunk.... /be
Fix checked into trunk. I'm setting the target milestone optimistically to 1.8rc1 based on comment 16.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Flags: blocking1.8rc1?
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8rc1
Not going to block on this but please request approval for the patch if it shakes out well on the trunk.
Flags: blocking1.8rc1? → blocking1.8rc1-
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: