Bug 48377
| Summary: | REGRESSION: r69429-r69611: Javascript parse and reference errors on Amazon Lightning deals | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Kevin M. Dean <kevin> |
| Component: | Evangelism | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | ap, dglazkov, edwardjsabol, evan, ggaren, jwalden+bwo, oliver, zherczeg |
| Priority: | P1 | Keywords: | InRadar |
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | http://www.amazon.com/gp/goldbox/ref=cs_top_nav_gb27 | ||
Kevin M. Dean
When viewing the link above (Amazon's Today's Deals link), the Lightning deals box at the upper right no longer appears.
Under r69611, the Web Inspector console reads:
SyntaxError: Parse error gbld_js-v19._V198980857_.js:4612
ReferenceError: Can't find variable: GBLD ref=cs_top_nav_gb27:2354
Under r70429, the Web Inspector console reads:
ReferenceError: Strict mode forbids implicit creation of global property 'Deal' gbld_js-v19._V198980857_.js:371
ReferenceError: Can't find variable: Deal ref=cs_top_nav_gb27:2397
This may be related to a crash issue that I'm also seeing between the same webkit nightly releases at Bug 47573 .
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Geoffrey Garen
<rdar://problem/8603250>
Oliver Hunt
Amazon is providing incorrect strict mode code.
Oliver Hunt
This isn't a regression, this is a progression.
Kevin M. Dean
Incorrect strict mode or not, it renders in every other browser fine, so why does it break here?
Oliver Hunt
(In reply to comment #4)
> Incorrect strict mode or not, it renders in every other browser fine, so why does it break here?
Because no other browser implements strict mode yet.
Kevin M. Dean
But isn't there a more elegant fallback than erroring out and not rendering anything, or is it pretty much all or nothing to do it right?
Kevin M. Dean
I guess it doesn't matter much for me at the moment since I can't use the recent nightlies anyway due to the constant crashing bug that I mentioned in my other Bug report. I'd really like to see that one fixed in case it has some effect on this one.
Oliver Hunt
(In reply to comment #6)
> But isn't there a more elegant fallback than erroring out and not rendering anything, or is it pretty much all or nothing to do it right?
No. It's not that we're choosing not to render their content, the problem is that they are attempting to build up the page from javascript and that javascript is wrong. We can't choose not to follow the spec a fundamental behaviour in the spec.
Zoltan Herczeg
Did you notify Amazon about this issue? What is their opinion?
Oliver Hunt
(In reply to comment #9)
> Did you notify Amazon about this issue? What is their opinion?
W#e don't have any contacts inside amazon that i'm aware of. The reality of matter is that their opinion isn't important. Their code is wrong. To make it work we would have to remove strict mode entirely which would basically neuter ES5.
Oliver Hunt
(In reply to comment #9)
> Did you notify Amazon about this issue? What is their opinion?
And this will also break in firefox nightlies in the near future (if it hasn't already)
Kevin M. Dean
I think it's more about informing Amazon that they have a broken implementation that they need to fix rather than having them asking Webkit to change. I wouldn't know who to contact there either. Would webmaster@amazon.com be an old-school guess?
Kevin M. Dean
(In reply to comment #11)
>
> And this will also break in firefox nightlies in the near future (if it hasn't already)
I just checked Minefield under Windows (Can't run the latest on my PPC Mac anymore) and you're right it's broken there as well.
Evan Martin
I have engineer friends at Amazon. I'll point them at this bug and see if they can route it to the appropriate people.
Zoltan Herczeg
> W#e don't have any contacts inside amazon that i'm aware of. The reality of matter is that their opinion isn't important. Their code is wrong. To make it work we would have to remove strict mode entirely which would basically neuter ES5.
Eh bugzilla... That question was not directed to you :)
Oliver Hunt
(In reply to comment #14)
> I have engineer friends at Amazon. I'll point them at this bug and see if they can route it to the appropriate people.
Any luck? Afaict we don't have any contacts at amazon :-(
Evan Martin
He said he forwarded it. Maybe they're looking at this conversation right now!
Oliver Hunt
(In reply to comment #17)
> He said he forwarded it. Maybe they're looking at this conversation right now!
dun dun duhhhhh!!!!
Dimitri Glazkov (Google)
(In reply to comment #18)
> (In reply to comment #17)
> > He said he forwarded it. Maybe they're looking at this conversation right now!
>
> dun dun duhhhhh!!!!
Can I get a free Kindle?
Kevin M. Dean
(In reply to comment #14)
> I have engineer friends at Amazon. I'll point them at this bug and see if they can route it to the appropriate people.
Evan, did you ever hear anything from Amazon?
This issue also affected Amazon's special Black Friday Week / Cyber Monday Lightning Deal pages.
Oliver Hunt
(In reply to comment #20)
> (In reply to comment #14)
> > I have engineer friends at Amazon. I'll point them at this bug and see if they can route it to the appropriate people.
>
> Evan, did you ever hear anything from Amazon?
>
> This issue also affected Amazon's special Black Friday Week / Cyber Monday Lightning Deal pages.
Have you reported the bug to amazon?
Kevin M. Dean
(In reply to comment #21)
> Have you reported the bug to amazon?
Myself, no. I don't have any contacts there, but Evan said he did so I was just following up.
Alexey Proskuryakov
*** Bug 50436 has been marked as a duplicate of this bug. ***
Evan Martin
I pinged my Amazon contact again. I'll let you know if he responds.
Edward Sabol
I understand not wanting to "neuter" ES5's strict mode, but I think this bug should be reopened with the component set to Evangelism. I filed a duplicate because I didn't find this "resolved" bug when I searched for both "Amazon" and "Lightning".
Alexey Proskuryakov
Indeed.
Evan Martin
Amazon contact says they're planning to work on it in January.
Personally, I think you should just blacklist Amazon from strict mode, but I'm into ugly hacks like that. I.e. any page on amazon.com that requests strict mode they should instead get a console.log() that prints out a link to this bug. That doesn't punish their users, but it will quickly get the attention of any developers who work on their site.
Oliver Hunt
(In reply to comment #27)
> Amazon contact says they're planning to work on it in January.
>
> Personally, I think you should just blacklist Amazon from strict mode, but I'm into ugly hacks like that. I.e. any page on amazon.com that requests strict mode they should instead get a console.log() that prints out a link to this bug. That doesn't punish their users, but it will quickly get the attention of any developers who work on their site.
JavaScriptCore is independent of web pages, it's a large amount of work to add infrastructure to say "ignore the standard in this case", and it's counter productive.
Logging an error rather than entering strict mode essentially makes it impossible for amazon to actually check that their site is strict mode compliant when they do get round to trying fix it. It's also a rather passive-aggressive response.
Evan Martin
I was just trying to suggest productive alternatives to the current behavior, which is failing to render their site. But it's your call.
Kevin M. Dean
Lightning Deals are now appearing correctly for me in webkit.
Alexey Proskuryakov
Thanks for the update!
Jeff Walden (remove +bwo to email)
https://bugzilla.mozilla.org/show_bug.cgi?id=627531
Looks like Amazon fixed only the particular problem here and didn't address the root problem: that concatenating coordinating strict and non-strict scripts is no longer safe. Is this second problem the only one that remains? Who knows. :-\
I suspect you want a new bug on this, but I'll leave that to someone else to file in case I'm wrong -- just noting it since I suspect people here would want to know about it.