Closed Bug 702847 Opened 13 years ago Closed 11 years ago

strange issue with array elements being undefined and testing typeof failing

Categories

(Add-on SDK Graveyard :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: steve, Unassigned)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:8.0) Gecko/20100101 Firefox/8.0 Build ID: 20111104165243 Steps to reproduce: I develop Reddit Enhancement Suite, which can be downloded from Github here: http://github.com/honestbleeps/Reddit-Enhancement-Suite I can run the following steps: - get code from github - cfx xpi to compile - go to www.reddit.com - click any user's "tag" icon, and set a tag along with a color (be sure to set a color)... - save the tag... then click the tag again to edit... you will get a javascript error. Actual results: I have a function in the code that takes in a SELECT form element and a value, and appropriately sets the selectedIndex based on that value -- like so: setSelectValue: function(obj, value) { console.log('set select value... ???!!?@#?!@#'); for (var i=0, len=obj.length; i < len; i++) { if ((typeof(obj[i]) != 'undefined') && (obj[i].value == value)) { obj[i].selected = true; } } }, Note that I have added in checks for typeof(obj[i]) - which really shouldn't even have to be checked! These should never be undefined. However, they are showing up as undefined in Firefox (and no other browser)... Furthermore, the check fails anyway, and the script fails! However, if I add a console.log(typeof(obj[i])) prior to the test -- then the test passes... this is consistent: remove that console.log, the script errors out again saying obj[i] is undefined. Expected results: What should have happened is that no elements in obj[] should be undefined ever! Even if they are, however, a check for typeof(obj[i]) should properly at least short circuit that if statement and not cause a javascript error. Worse yet, throwing in a console.log statement prior to that check of: console.log(typeof(obj[i])) suddenly at least avoids a javascript error. removing the console.log statement makes that error come back. I will try and create a more isolated test case if and when I have more time, but I'm not sure how soon I can get to that, so I wanted to link to the full project for you to at least be able to easily download and replicate the steps. Thanks!
Attached patch Possible fix (deleted) — Splinter Review
Oh there is a known issue with html collections in content script and Select is one of them, I didn't knew that! Steve, can you try your addon with this patch applied? This patch is against current master and won't work if you are using 1.2/1.3 releases.
Assignee: nobody → poirot.alex
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Without this patch I had an exception from this line: https://github.com/honestbleeps/Reddit-Enhancement-Suite/blob/master/XPI/data/reddit_enhancement_suite.user.js#L6321 With it, I don't get any exception anymore, but color is still not updated in select ... can you confirm that there is no more issue around select?
Sorry for the slow response - I haven't yet had the chance to try out the patch, but I'm about to investigate getting current master and wanted to let you know I haven't abandoned tracking this bug, just been busy!
Update: It appears that that patch fixes it! I didn't have the same problem Alexandre had with the color not being selected... it works for me. Thank you!
Comment on attachment 574862 [details] [diff] [review] Possible fix Ok so if that fix this bug, let's review it! In this patch, I just applied the same trick as with NodeList and HTMLCollection, for an unknown reason there is an issue around lists. You can access to each list elements only once, then it becomes undefined :x The best would be to figure out what is really going on, but the only way to digg this is by debugging it through gdb and it will take lot of time :( I already tried to reproduce this bug with simple test case without success. I don't think we can reproduce it without using Proxies.
Attachment #574862 - Flags: review?(rFobic)
gabor: By any chance, would you have any guess why we have such issue ? Do you know anything special around NodeList, HtmlCollection & all ?
(In reply to Alexandre Poirot (:ochameau) from comment #6) > gabor: By any chance, would you have any guess why we have such issue ? > Do you know anything special around NodeList, HtmlCollection & all ? No, I don't know much about this part, but me or Eddy will take a look at it hopefully soon. Shall we add this bug or it's meta bug to bug 697775 ?
I'm not 100% sure there is something wrong in platform code, it may just be something wrong in my proxy code. But in order to be sure, we will have to see what gdb says about these undefined values.
Comment on attachment 574862 [details] [diff] [review] Possible fix Review of attachment 574862 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #574862 - Flags: review?(rFobic) → review+
(In reply to Alexandre Poirot (:ochameau) from comment #6) > gabor: By any chance, would you have any guess why we have such issue ? > Do you know anything special around NodeList, HtmlCollection & all ? I think they all are lazily populated, also jsdom project needed proxies for that.
Assignee: poirot.alex → ejpbruel
I can reproduce this bug on Firefox 8, but not on Firefox Nightly, Aurora or Beta. I think it's safe to assume that this issue has been fixed, even though we do not know what the cause of the problem was.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Interesting, I give it a try too and it appears that the whole class of similar issue. i.e. all issues around Collection have been fixed. So that we should follow this in order to remove workarounds when we do no support buggy firefox versions! So Beta means the next one, 9, right? So we will be able to remove them when we drop 8 support. It would be handy to know on which jetpack release it will happened so that we can drop some notes somewhere?
Firefox 9 comes out on December 20, so if I understand our support and release plans, master branch will switch to supporting Firefox 9 through Nightly 12 three weeks after December 20. So January 10?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hmm, I thought we decided that master's minVersion will get bumped in conjunction with the Firefox release, so next week.
Assignee: ejpbruel → nobody
I think this is no longer an issue and the above has landed; closing it up!
Status: REOPENED → RESOLVED
Closed: 13 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: