Setting indexed properties on HTMLSelectElement to null no longer works

VERIFIED FIXED in Firefox 15

Status

()

Core
DOM
VERIFIED FIXED
5 years ago
5 years ago

People

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

Tracking

({regression})

Trunk
mozilla17
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14 unaffected, firefox15+ verified, firefox16+ verified, firefox17+ verified, firefox-esr10 unaffected)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 649789 [details]
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.

Comment 1

5 years ago
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.
(Reporter)

Comment 3

5 years ago
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>
Created attachment 650160 [details]
Testcase from comment 3
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; }
(Reporter)

Comment 6

5 years ago
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.
Regression range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c758cc9b60e5&tochange=ac968ff4fe41
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
Created attachment 650174 [details]
Minimal testcase.  The select should say "Correct"
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
(Reporter)

Comment 11

5 years ago
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.  ;)
(Assignee)

Comment 13

5 years ago
Created attachment 650186 [details] [diff] [review]
Patch v1

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+
(Assignee)

Comment 15

5 years ago
Created attachment 650277 [details] [diff] [review]
Patch v2

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+
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/mozilla-central/rev/cefda2b7296b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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
(Assignee)

Comment 18

5 years ago
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?
tracking-firefox15: ? → +
tracking-firefox16: ? → +
tracking-firefox17: ? → +
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+
(Assignee)

Comment 20

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/648e60750d53
https://hg.mozilla.org/releases/mozilla-beta/rev/50f5c2689179
status-firefox15: affected → fixed
status-firefox16: affected → fixed
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
You need to log in before you can comment on or make changes to this bug.