Bug 50483
Summary: | REGRESSION: Event calendar fails to load at http://ipfw.edu/calendar/ | ||
---|---|---|---|
Product: | WebKit | Reporter: | Barton L. Tyner Jr. <barton.tyner> |
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ap, barraclough, by, dimitris.apostolou, dmandelin, ggaren, msaboff, rvjansen, simon.fraser, tobiaslidskog |
Priority: | P1 | Keywords: | Regression |
Version: | 528+ (Nightly build) | ||
Hardware: | Mac (Intel) | ||
OS: | OS X 10.6 | ||
URL: | http://ipfw.edu/calendar/ |
Barton L. Tyner Jr.
Using Webkit 534.13+, the calendar fails to draw or present any content. The calendar worked fine in previous versions of Webkit, and it shows up fine in the latest versing of Safari.
The calendar depends heavily on JavaScript.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
I can't reproduce this with a local debug build of r72777, but I can with r73316 nightly - looks like a very recent regression.
SyntaxError: Invalid regular expression: invalid range in character class
Gavin Barraclough
Hi Barton, Alexey,
This will likely have been caused by:
https://bugs.webkit.org/show_bug.cgi?id=48100
which made our regex syntax checking slightly more stringent, and ECMA-262 compliant.
We should check the regexes in this page, and see whether they comply to the spec – I'm guessing that one is invalid (specifically, contains an invalid range in a character class - one containing a character class escape, e.g. /[d-\d]/)
Assuming that this is the case, I'm not sure not to proceed.
Option A - Hope that this is not a major cause of problems, and that we can continue to move in the direction of greater spec compliance. I think a good first course of action would be to treat this as an advocacy issue – we should reach out to IPFW, and recommend that they update the problematic source to be ECMA-262 compliant (this will probably just involve adding an extra missing '\' - under ECMA-262 something like /[d-\d]/ contains an invalid range, but you can just escape the hyphen to turn this into a valid regular expression - /[d\-\d]/. This should match our prior behavior). If IPFW change their code & no more similar issues arise, this could be a good resolution to the problem.
Option B - Retreat from spec compliance on this matter, revert this change, and settle for noncompliant behavior for now (and raise the matter in TC-39 with a view to hopefully improving the future revisions of the spec).
I'd suggest we start by sticking with option A, since reverting the change immediately loses the ability to track whether or not further dupes come in, and this is a broader problem. I'd like to strongly believe in complying to the spec on this matter, but since the spec disallows a number of further regex features (e.g. octal escapes) that we don't believe we'll be able to remove without breaking the web I don't think 100% compliance is considered a realistic goal here anyway.
cheers,
G.
Alexey Proskuryakov
But does the same regex work in both IE and Firefox? Are we getting incompatible with both?
Barton L. Tyner Jr.
Thanks for the thoughtful response to this issue.
We should be able to fix the escape issue on our end.
Something similar seems to be happening on the TED site. For example, when you go to http://www.ted.com/tedx/events/1758.html, the page fails to draw like the ipfw.edu/calendar page. However, unlike the ipfw.edu/calendar page, the TED page will draw when you do a forced refresh.
Thanks,
Bart
Alexey Proskuryakov
One ted.com issue is tracked as bug 50526.
Gavin Barraclough
Okay, that sounds great. Thanks for the bug report Bart! - & we'll keep tabs on how broad an issue this is, & see if the TED issue can also be resolved through advocacy.
cheers,
G.
Alexey Proskuryakov
See also: bug 50538.
Gavin Barraclough
Looks like this change was breaking more sites, so we've largely reverted this change in r73594, related bugs should be fixed.
Behavior post r73594 should broadly match that before pre r72813, with one exception. While we're no longer generating the syntax errors required by the ECMA-262 spec, we will now only allow ranges permitted by the grammar. Likely impact is low, but would be visible is expressions such as /[\d-a-z]/.
before r72813 this would be handled as equivalent to /[\d\-a-z]/, i.e a-z will be treated as a range. Now this will be treated as /[\d\-a\-z]/, which at least complies with the grammar in the spec.
Gavin Barraclough
*** Bug 50538 has been marked as a duplicate of this bug. ***
Gavin Barraclough
*** Bug 50654 has been marked as a duplicate of this bug. ***
Gavin Barraclough
*** Bug 50646 has been marked as a duplicate of this bug. ***
Andy Estes
*** Bug 50320 has been marked as a duplicate of this bug. ***
Dave Mandelin
I had to look into this issue recently while updating Yarr in Firefox. I found that we have test cases that are a little bit stricter than what Yarr currently supports. Namely, we consider these to be syntax errors:
[C-\\s]
[C-\\d]
[C-\\W]
[\\b-\\w]
[\\B-\\w]
(See also https://bugzilla.mozilla.org/show_bug.cgi?id=576837)
Apparently, this is fine for web compat, or at least we haven't received any complaints about it yet. I have a small patch for this if anyone is interested.
Gavin Barraclough
*** Bug 50755 has been marked as a duplicate of this bug. ***