Closed Bug 780993 Opened 12 years ago Closed 12 years ago

Setting indexed properties on HTMLSelectElement to null no longer works

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla17
Tracking Flags:
Tracking Status
firefox14 --- unaffected
firefox15 + verified
firefox16 + verified
firefox17 + verified
firefox-esr10 --- unaffected

People

(Reporter: john.o.nye, Assigned: Ms2ger)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Archive.zip
10.08 KB, application/octet-stream
Details
Testcase from comment 3
2.46 KB, text/html
Details
Minimal testcase. The select should say "Correct"
181 bytes, text/html
Details
Patch v2
3.52 KB, patch
bzbarsky
: review+
lsblakk
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file Archive.zip — 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:15.0) Gecko/20100101 Firefox/15.0
Build ID: 20120717110313

Steps to reproduce:

I am writing routines to fill a form and want to clear a select that may contain residue from prior use. The routine contains a while statement to empty the select options. Today Firefox loaded an update and the function does not complete and no error is shown in the Firebug console. 


Actual results:

The form should have been cleared and then populatd with the entry to be edited. It cleared the text box, but did not clear the list and exited without running functions to display control buttons


Expected results:

The clear should complete, the form should be repopulated and the buttons displayed. The attached ZIP contains the html, css and js files. The steps taken: select text response creation. Save a completed entry. highlight the new entry in righthand list and click edit. population does not complete.
The title claims that "javascript while statement exits function". Do you have a smaller example that exhibits the problem?
Actually, the given testcase might be OK, but what's really needed are actual steps to reproduce (and actual and expected results) using that testcase.  That is, exact instructions on what to click in what order and how someone not familiar with what the page is _trying_ to do would tell whether things were cleared correctly.
I have condensed the conditions producing the error into a small html file. It has one button that performs the actions leading to the error. - It is copied below:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>
<head>
<meta http-equiv="Content-type" Content="text/html; charset=UTF-8">
<title>Bug Page</title>
<style>
body, select, text, textarea, input, select { font: normal normal 11px Verdana, Geneva, Arial, Helvetica, sans-serif; }
span.reqset { color: #dc143c; font-size: 9px; }
span.clickme { cursor:pointer }
th.stmt { width: 200px; }
th.rate { width: 100px; }
tr.sepa { height: 2px; }
td.sepa { height: 2px; background: lightgray; }
</style>
<script language=javascript>

var toedit = 'Page-A:-:Text:-:New Text:-:New One:=:New Two';

function checkSize (tel, size) {
	var fm = document.forms[0];
	var thisCont = fm.elements[tel].value;
	var thisLen = thisCont.length;
	if (thisLen > size) {
		thisCont = thisCont.substr(0, size);
		thisLen = size;
		fm.elements[tel].value = thisCont;
	}
	var dsize = size - thisLen;
	document.getElementById(tel + 'X').innerHTML = dsize;
}
function setSelSize (tel) {
	var fm = document.forms[0];
	var theSize = fm.elements[tel].length;
	document.getElementById(tel + 'X').innerHTML = theSize;
}
function edpage() {
	alert('Starting form stup');
	document.forms[0].tptc.value = '';
	alert('Clearing stuff');
	while(document.forms[0].tell.length > 0) { document.forms[0].tell[0] = null; }
	alert('Have cleared the list');
	if (toedit != '') {
		alert('Have to prime form');
		var tArr = toedit.split(':-:');
		document.forms[0].tptc.value = tArr[2].replace(/<br>/g,'\n');
		var tLi = tArr[3].split(':=:');
		for (var ix = 0; ix < tLi.length; ix++) {
			document.forms[0].tell.add(new Option(tLi[ix], tLi[ix]), null);
		}
		document.forms[0].tell.size = 2;
		if (document.forms[0].tell.length > 2 && document.forms[0].tell.length < 10) document.forms[0].tell.size = document.forms[0].tell.length;
		if (document.forms[0].tell.length > 9) document.forms[0].tell.size = 9;
		
		alert('Set Form Sizes');
		checkSize('tptc',500);
		alert('Set Header Size');
		setSelSize('tell');
	}
}
function sendA(msg) {
	alert(msg);
}

</script>
</head>
<body>
<form>
<textarea wrap=logical name=tptc cols=100 rows=5 onKeyUp="sendA('KeyUp'); checkSize('tptc',500);">Residue text</textarea><span id=tptcX>488</span><br>
<select style="width:550px" size=2 name=tell>
<option value="Old One">Old One</option>
<option value="Old Two">Old Two</option>
</select><span id=tellX>2</span><br>
<input type=button value="Reload" onClick="sendA('Reload'); edpage();">
</form>
</body>
</html>
Running that testcase and clicking the "Reload" button (which I assume is what one is supposed to do), I get an exception thrown on this line:

  while(document.forms[0].tell.length > 0) { document.forms[0].tell[0] = null; }
Yes, that is the problem I am reporting. I see this in Firefox on OSX Build 15, with Web Console reporting an NS_ERROR_UNEXPECTED: Unexpected error

When I run in Chrome or Safari - no error is experienced.

The code has been static in this area for some time, and the error appeared a few days ago with a Firefox Automatic Update.
This has got to be a regression from bug 752226.  Still looking for the exact codepath that was changed incorrectly there.
Assignee: general → nobody
Status: UNCONFIRMED → NEW
Component: JavaScript Engine → DOM
Ever confirmed: true
Summary: javascript while statement exits function → Setting indexed properties on HTMLSelectElement to null no longer works
nsHTMLSelectElementSH::SetOption is what used to accept null and no longer does.

The good news is that this is only on beta so far.  John, thank you very much for running betas and for reporting the bug!

Ms2ger is going to take this.
Blocks: 752226
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
Keywords: regression
I am glad to be able to help. I am truly impressed with your response time. I am used to dealing with Oracle Support which tends to be slow to react.
Yeah, we do try to be better than Oracle.  ;)
Attached patch Patch v1 (obsolete) — — Splinter Review 
https://tbpl.mozilla.org/?tree=Try&rev=826453a14ec5
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
Attachment #650186 - Flags: review?(bzbarsky)
Comment on attachment 650186 [details] [diff] [review]
Patch v1

r=me
Attachment #650186 - Flags: review?(bzbarsky) → review+
Attached patch Patch v2 — — Splinter Review 
This one doesn't crash... do_QueryWrapper isn't null-safe, unlike all other do_*s.
Attachment #650186 - Attachment is obsolete: true
Attachment #650277 - Flags: review?(bzbarsky)
Comment on attachment 650277 [details] [diff] [review]
Patch v2

<sigh>.

r=me
Attachment #650277 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/cefda2b7296b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
status-firefox-esr10: --- → unaffected
status-firefox14: --- → unaffected
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → fixed
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Version: 15 Branch → Trunk
Comment on attachment 650277 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 752226
User impact if declined: exceptions are thrown on some web pages
Testing completed (on m-c, etc.): On m-c; passes tests.
Risk to taking this patch (and alternatives if risky): Little risk
String or UUID changes made by this patch: None
Attachment #650277 - Flags: approval-mozilla-beta?
Attachment #650277 - Flags: approval-mozilla-aurora?
Comment on attachment 650277 [details] [diff] [review]
Patch v2

Since this has tests and first appears in 15, we'll take this for branches.
Attachment #650277 - Flags: approval-mozilla-beta?
Attachment #650277 - Flags: approval-mozilla-beta+
Attachment #650277 - Flags: approval-mozilla-aurora?
Attachment #650277 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Able to see the issue on FF 15b4.
No errors on FF 15b5 in the web console using the STR in comment 5. Also, the "correct" value is displayed loading the testcase in comment 9.
Verified fixed on FF 15b5 on Win 7 x64, Ubuntu 12.04 and Mac OS X 10.6.
status-firefox15: fixed → verified
Keywords: verifyme
QA Contact: paul.silaghi
Paul, please verify this against the latest Firefox 16 beta and Firefox 17 aurora, thanks.
Keywords: verifyme
Verified fixed on FF 16b3 and FF 17.0a2 (2012-09-18) on Win 7 x64, Ubuntu 12.04 and Mac OS X 10.7.4
Status: RESOLVED → VERIFIED
status-firefox16: fixed → verified
status-firefox17: fixed → verified
Keywords: verifyme
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.