599854 - Drop down list is empty on certain site

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alice0775 White]

Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100927 Firefox/4.0b7pre ID:20100927041306

Drop down list is empty.

An error is displayed in the Error Console.

Error: messages is not defined
Source file: http://www.usa.canon.com/cusa/support/consumer/scanners/canoscan_series/canoscan_lide25?selectedName=DriversAndSoftware
Line: 2

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile.
2. Open URL, (ignore popup).
3. Click Drop down marker "Select OS" of Drivers & Software paragraph.

Actual Results:
 Dropdown list is empty.

Expected Results:
 Dropdown list should be displayed as follows.
 Select OS
 Windows Vista
 Windows 7
 .
 .
 .
 Mac OS X
 Windows XP(x64)


Regression window for m-c build:
Works:
http://hg.mozilla.org/mozilla-central/rev/0ee09dea0911
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre ID:20100813140349
Fails:
http://hg.mozilla.org/mozilla-central/rev/fc6783c960ca
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre ID:20100813142423
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0ee09dea0911&tochange=fc6783c960ca


Regression window for TM build:
Works:
http://hg.mozilla.org/tracemonkey/rev/f84b470314a8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100811 Minefield/4.0b4pre ID:20100811105635
Fails:
http://hg.mozilla.org/tracemonkey/rev/ab1c626399bb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre ID:20100812043145
Pushlog:
http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=f84b470314a8&tochange=ab1c626399bb

Candidate bug:
Bug 564953 - (PortYarr) JM: Port YARR!

Comment 1

[Alice0775 White]

This problem happens on Linux too.
Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100927 Firefox/4.0b7pre ID:20100927031227

Comment 3

[Cork]

Dug into this looking into what happens in bug 601065, here's what i found:

It uses dojo to make an ajax call for the file url: http://undefined:[e-mail]/wsss/GeneratedPages/canonUSA/DownloadContents/English/0307B.html

this file in turn includes two javascript files:
<script type="text/javascript" src="/wsss/GeneratedPages/wsss_xsl/xsl-dep.js">//empty
</script>
<script type="text/javascript" src="/wsss/GeneratedPages/wsss_xsl/default.js">//empty
</script>

These files is never requested, so a bunch of variables and functions never gets declared.

Comment 4

[Chris Leary [:cdleary] (not checking bugmail)]

Looking now. If it is YARR, this is the likely culprit, in dojo's dijit ContentPane._setContent

	match = cont.match(/<body[^>]*>\s*([\s\S]+)\s*<\/body>/im);

Comment 5

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: Test case (without text data).

Old TM engine and V8 agree on the result, I'm betting this is a bug in our PCRE extensions.

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: Text data.

Comment 7

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: Reduced test case.

Comment 8

[Chris Leary [:cdleary] (not checking bugmail)]

Yup, it's our PCRE modifications, JSC doesn't have the same issue.

Comment 9

[David Mandelin [:dmandelin]]

What's the status here?

Comment 12

[David Mandelin [:dmandelin]]

Further reduced test case:

  var re = /(?:(?:(")(c)")?)*/;
  var text = '"c"';
  print(re.exec(text));

Expected output: "c",",c
Actual output:   "c",,

Both quantifiers (? and *) are required for the test to fail: if either is removed, it runs according to spec.

Comment 13

[David Mandelin [:dmandelin]]

Attachment: WIP

This fixes the reduced test case, but not the original. Here is another small test case that also fails with this patch:

  var re = /(?:a*?(?:(")(c)")?)*/;
  var text = '"c"';
  print(re.exec(text));

In this bug, and also bug 613399, the basic problem is the handling of quantified matches of the empty string.

Comment 14

[David Mandelin [:dmandelin]]

(In reply to comment #13)
> In this bug, and also bug 613399, the basic problem is the handling of
> quantified matches of the empty string.

I think the key is consistent enforcement of "NOTE 4" in ECMA-262 15.10.2.5, which is that once the minimum number of repetitions of a quantified subexpression has been satisfied, any further matches of it that match the empty string are not considered. This is one of the subtler corners of ECMA-262 regexes. This rule (really, an implicit feature of the continuation-based spec) is important both to avoid infinite loops and to avoid "capturing" undefined after capturing some nonempty values.

In the current (tm tip) tree version of PCRE, the rule is maintained for the case where we are ending a captured group (KET* with index > 0) and we have groupMatched == true (meaning we were called for BRAZERO (i.e., ? over the current group) or KET* continuation (i.e., trying to match more copies). But we are missing enforcing the rule for:

 - noncaptured groups in the same situation (that's my WIP above)

 - ?-quantified stuff. If a BRAZERO guards some stuff that matches empty, that
   should not be considered a match of the interior part. This can be seen with
   the test case /()?/.exec(""), which currently returns ["", ""] but should
   return ["", (void 0)].

I think fixing these two should take care of this.

Comment 16

[David Mandelin [:dmandelin]]

Attachment: Patch

Comment 17

[David Mandelin [:dmandelin]]

Notes on the patch: it's more 0-length repeat match stuff. First, we have to check for that in KET in the non-capturing case. Second, we need to propagate that groupMatched flag "across" quantified single-character matches.

We should audit the setting of groupMatched some more after this, but I first want to fix this bug, as I am pretty sure this patch is correct. I think the general concept is this:

1. groupMatched is *used* only in KET*, to detect that we are matching something 0-length when we are trying to match more repeats for a KETRMIN or KETRMAX.
2. it should be "set" (i.e., given a literal value in a "recursive" (i.e., backtrackable) match invocation) only in something that matches a KET, because KET is what uses it. Otherwise, it should be propagated from the starting operator at the same level
3. thus, "starting matching" and ASSERT should pass false, while BRAZERO, KETRMAX/KETRMIN should pass true (they are exactly the cases where zero-length matches don't count). Everything else should propagate.

Comment 19

[Chris Leary [:cdleary] (not checking bugmail)]

Comment on attachment 503714 [details] [diff] [review]
Patch

This fixes the regression, although we recognized bug 627609, which we still have to fix.

Comment 20

[David Mandelin [:dmandelin]]

http://hg.mozilla.org/tracemonkey/rev/ccd420e49864

Comment 21

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/ccd420e49864

Comment 22

[Alice0775 White]

Confirmed.
This was fixed.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110121 Firefox/4.0b10pre ID:20110121030329