Closed
Bug 786114
Opened 11 years ago
Closed 11 years ago
Firefox 17 doesn't display graphs anymore on Khan Academy (recent regression)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla18
People
(Reporter: epinal99-bugzilla2, Assigned: n.nethercote)
References
Details
(Keywords: regression)
Attachments
(3 files)
5.15 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
2.46 KB,
patch
|
luke
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
7.77 KB,
patch
|
luke
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Reporter: http://forums.mozillazine.org/viewtopic.php?p=12231281#p12231281 Open http://www.khanacademy.org/math/algebra/algebra-functions/e/graphing_points_2 Graph is missing. m-c good=2012-08-26 bad=2012-08-27 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b3cce81fef1a&tochange=8af6a22827ec
Keywords: regression
![]() |
||
Comment 1•11 years ago
|
||
Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/0a9e931cdcf3 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120826192258 Bad: http://hg.mozilla.org/mozilla-central/rev/8af6a22827ec Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120826222258 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0a9e931cdcf3&tochange=8af6a22827ec Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/74666a1eb791 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120826130359 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/d178a49c979c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120826160958 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=74666a1eb791&tochange=d178a49c979c Suspected: Bug 784608
Assignee: nobody → general
Blocks: 784608
tracking-firefox17:
--- → ?
tracking-firefox18:
--- → ?
Component: General → JavaScript Engine
![]() |
||
Comment 2•11 years ago
|
||
First bad : 660d18ddffcd Nicholas Nethercote — Bug 784608 (part 7) - Change the form and meaning of ParseContext::innermostWith, and do follow-up simplifications. r=luke.
![]() |
Assignee | |
Updated•11 years ago
|
Assignee: general → n.nethercote
![]() |
Assignee | |
Comment 3•11 years ago
|
||
In http://hg.mozilla.org/integration/mozilla-inbound/rev/660d18ddffcd it's the change to needsImplicitThis() that's the problem.
Updated•11 years ago
|
Updated•11 years ago
|
![]() |
Assignee | |
Comment 4•11 years ago
|
||
Nested functions that needs implicit |this| within an eval() weren't being handled correctly. This patch adds handling for that -- it's the third case in FunctionBox(), which was missing. The patch also adds a bunch of comments to FunctionBox() to better explain what's going on. And it adds a test. ps: The JS code at www.khanacademy.org is an abomination.
Attachment #656721 -
Flags: review?(luke)
![]() |
Assignee | |
Comment 5•11 years ago
|
||
Let's just back out the regressing patch on Aurora. This is the patch that followed it, and so must be backed out as well.
Attachment #656737 -
Flags: review?(luke)
![]() |
Assignee | |
Comment 6•11 years ago
|
||
This is the backout of the regressing patch.
Attachment #656739 -
Flags: review?(luke)
![]() |
Assignee | |
Comment 7•11 years ago
|
||
Comment on attachment 656737 [details] [diff] [review] Backout d178a49c979c (bug 785608 part 8) for regressing |with| handling. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 784608 User impact if declined: some incorrect JS behaviour Testing completed (on m-c, etc.): just backs out the regressing patch Risk to taking this patch (and alternatives if risky): none String or UUID changes made by this patch: none
Attachment #656737 -
Flags: approval-mozilla-aurora?
![]() |
Assignee | |
Comment 8•11 years ago
|
||
Comment on attachment 656739 [details] [diff] [review] Backout 660d18ddffcd (bug 785608 part 7) for regressing |with| handling. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 784608 User impact if declined: some incorrect JS behaviour Testing completed (on m-c, etc.): just backs out the regressing patch Risk to taking this patch (and alternatives if risky): none String or UUID changes made by this patch: none
Attachment #656739 -
Flags: approval-mozilla-aurora?
![]() |
||
Comment 9•11 years ago
|
||
Comment on attachment 656721 [details] [diff] [review] Fix handling of nested functions that need implicit |this| within eval(). Review of attachment 656721 [details] [diff] [review]: ----------------------------------------------------------------- Nice testcase and comments. 'with' is some piece of work. ::: js/src/frontend/Parser.h @@ +188,5 @@ > bool parsingForInit:1; /* true while parsing init expr of for; > exclude 'in' */ > bool parsingWith:1; /* true while we are within a > with-statement or E4X filter-expression > + in the current ParseContext chain trailing whitespace
Attachment #656721 -
Flags: review?(luke) → review+
![]() |
||
Updated•11 years ago
|
Attachment #656737 -
Flags: review?(luke) → review+
![]() |
||
Updated•11 years ago
|
Attachment #656739 -
Flags: review?(luke) → review+
![]() |
Assignee | |
Comment 10•11 years ago
|
||
> 'with' is some piece of work.
We should, like, totally ban it in strict mode or something.
![]() |
Assignee | |
Comment 11•11 years ago
|
||
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/325b276940b5 Still need approval to back the old patches out from Aurora.
Whiteboard: [leave open]
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/325b276940b5
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
![]() |
Assignee | |
Comment 13•11 years ago
|
||
The whiteboard says "[leave open]" -- we need to land the other patches on Aurora.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 14•11 years ago
|
||
Sorry, must've missed that. That said, that's not how we traditionally have dealt with back ports. AFAIK, we resolve a bug as soon as it hits mozilla-central and use the status-firefoxN flags to mark whether it's been fixed in Aurora/Beta/Release.
Updated•11 years ago
|
status-firefox17:
--- → affected
status-firefox18:
--- → fixed
Updated•11 years ago
|
Attachment #656737 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #656739 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
![]() |
Assignee | |
Comment 15•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b4b0c5e6f8db https://hg.mozilla.org/releases/mozilla-aurora/rev/3ab679cebfe4
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Updated•11 years ago
|
Comment 16•11 years ago
|
||
Site is not displaying correctly again. Practice work is broken, and the google map refuses to be usable. Nightly x64, 10/6/2012 build.
Comment 17•11 years ago
|
||
chaosfreedom1220, this bug is fixed. If you are sure it's a Firefox problem (happens with a new profile), file a new bug.
Reporter | ||
Comment 18•11 years ago
|
||
(In reply to chaosfreedom1220 from comment #16) > Site is not displaying correctly again. Practice work is broken, and the > google map refuses to be usable. > > Nightly x64, 10/6/2012 build. See bug 798834.
Comment 19•11 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0 beta 1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 beta 1 Verified on Firefox 17 beta 1 that the graphing math problems on khan academy are properly displayed on Windows 7 both 32 bit version and 64 bit version (verified also that other graphics from the left menu are also displayed correctly).
Comment 20•11 years ago
|
||
Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0b1
You need to log in
before you can comment on or make changes to this bug.
Description
•