957562 - Browser crashes on trying to modify the Graph values - WebGL Animations

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[svbabukumar]

Attachment: crashreport.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.72 Safari/537.36

Steps to reproduce:

Step 1: Launch the Aurora version of Browser (Build 28)
Step 2: Navigate to the URL "https://www.google.com/search?sourceid=chrome&ie=UTF-8&q=sqrt%28xx%2Byy%29%2B3cos%28sqrt%28xx%2By*y%29%29%2B5#q=sqrt(x*x%2By*y)%2B3*cos(sqrt(x*x%2By*y))%2B5"
Step 3: In the graph displayed try to change the value of the X,Y or Z by clicking on the fields


Actual results:

Browser crashes on clicking on the fields X,Y or Z


Expected results:

Browser should not result crash report instead it should allow user to modify the value of X,Y or Z

Comment 1

[Mayank Kumar [:mayanktg]]

This problem is related to the Graphics Card you are using. Firefox supports only newest drivers. This has already been discussed in the previous bugs as mentioned here https://wiki.mozilla.org/Blocklisting/Blocked_Graphics_Drivers#NVIDIA_cards

For more information visit this link
http://www.khronos.org/webgl/wiki/BlacklistsAndWhitelists Also check whether your Card's driver version is compatible with WebGL for Firefox or not.

Comment 2

[svbabukumar]

The same scenario is working fine in Live build 26 with same system graphics

Details of the crash / system are provided below

AdapterDeviceID: 0x2a42
AdapterVendorID: 0x8086
Add-ons: dojo%40silvergate.ar.ibm.com:1.2.0a1,eventbug%40getfirebug.com:0.1b10,fbtest%40mozilla.com:1.12b4,fbtrace%40getfirebug.com:1.12b3,firediff%40johnjbarton.com:1.2.1,FirePHPExtension-Build%40firephp.org:0.7.4,FireXPath%40pierre.tholence.com:0.9.7,info%40cssUpdater.com:0.5.2,netexport%40getfirebug.com:0.9b4,sroussey%40illumination-for-developers.com:1.1.26,crashme%40ted.mielczarek.org:0.3,%7B972ce4c6-7e08-4474-a285-3208198ce6fd%7D:28.0a2,firebug%40software.joehewitt.com:1.12.5
AvailablePageFile: 4047560704
AvailablePhysicalMemory: 1054490624
AvailableVirtualMemory: 3786219520
BIOS_Manufacturer: Dell Inc.
BlockedDllList: 
BreakpadReserveAddress: 50855936
BreakpadReserveSize: 37748736
BuildID: 20140108004006
CrashTime: 1389266226
EMCheckCompatibility: true
FramePoisonBase: 00000000f0de0000
FramePoisonSize: 65536
InstallTime: 1389266096
Notes: AdapterVendorID: 0x8086, AdapterDeviceID: 0x2a42, AdapterSubsysID: 04011028, AdapterDriverVersion: 8.15.10.2869
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ WebGL? EGL? EGL+ GL Context? GL Context+ WebGL+ 
ProductID: {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
ProductName: Firefox
ReleaseChannel: aurora
SecondsSinceLastCrash: 86339
StartupTime: 1389266096
SystemMemoryUsePercentage: 75
Theme: classic/1.0
Throttleable: 1
TotalVirtualMemory: 4294836224
URL: https://www.google.com/search?sourceid=chrome&ie=UTF-8&q=sqrt%28xx%2Byy%29%2B3cos%28sqrt%28xx%2By*y%29%29%2B5#q=sqrt(x*x%2By*y)%2B3*cos(sqrt(x*x%2By*y))%2B5
Vendor: Mozilla
Version: 28.0a2
Winsock_LSP: MSAFD Tcpip [TCP/IP] : 2 : 1 :  
 MSAFD Tcpip [UDP/IP] : 2 : 2 : %SystemRoot%\system32\mswsock.dll 
 MSAFD Tcpip [RAW/IP] : 2 : 3 :  
 MSAFD Tcpip [TCP/IPv6] : 2 : 1 : %SystemRoot%\system32\mswsock.dll 
 MSAFD Tcpip [UDP/IPv6] : 2 : 2 :  
 MSAFD Tcpip [RAW/IPv6] : 2 : 3 : %SystemRoot%\system32\mswsock.dll 
 RSVP TCPv6 Service Provider : 2 : 1 :  
 RSVP TCP Service Provider : 2 : 1 : %SystemRoot%\system32\mswsock.dll 
 RSVP UDPv6 Service Provider : 2 : 2 :  
 RSVP UDP Service Provider : 2 : 2 : %SystemRoot%\system32\mswsock.dll
useragent_locale: en-US

This report also contains technical information about the state of the application when it crashed.

Comment 3

[Ioana (away)]

Reproducible on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (20140109004001)
https://crash-stats.mozilla.com/report/index/dece4fc8-dd35-45ee-ad91-bc9092140109

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (20140109030203)
https://crash-stats.mozilla.com/report/index/4df2f230-0ace-4eaf-a4a5-e05b22140109

Not reproducible on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 (20140106141415)

Comment 4

[Ioana (away)]

This isn't about old graphics cards. I reproduced this issue with an Intel HD 4000.

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment 3 implies this is a recent regression. Can we get a regression window in that case?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This stack is consistent with the variable |frame| in mozilla::dom::HTMLInputElement::PreHandleEvent pointing to a deleted frame.  (this->mStyleContext yields a poisoned address, and DoGetStyleDisplay would be the first point in nsStyleContext code that touches the this pointer.)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I can easily reproduce this.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 9

[Mats Palmgren (inactive)]

Comment on attachment 8358202 [details] [diff] [review]
fix

There's one more possible occurrence of this near the end of the method:
http://hg.mozilla.org/mozilla-central/annotate/30f3710477c2/content/html/content/src/HTMLInputElement.cpp#l3456

The SetValueInternal call has calls to SetAttr with aNotify=true which
I believe can kill the frame so 'numberControlFrame' might be dead
on the line after.  In this case though,  the correct fix would be to
add "nsWeakFrame weakFrame(numberControlFrame);" on the line before
the SetValueInternal call and check that it's Alive() before calling 
numberControlFrame->HandlingInputEvent(false).  (fetching the primary
frame again and using that seems like it could do the wrong thing
in case the element was re-framed).

r=mats with that addressed (or dismissed if it's not an issue)

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Mats Palmgren (:mats) from comment #9)
> The SetValueInternal call has calls to SetAttr with aNotify=true which
> I believe can kill the frame so 'numberControlFrame' might be dead
> on the line after.  In this case though,  the correct fix would be to
> add "nsWeakFrame weakFrame(numberControlFrame);" on the line before
> the SetValueInternal call and check that it's Alive() before calling 
> numberControlFrame->HandlingInputEvent(false).  (fetching the primary
> frame again and using that seems like it could do the wrong thing
> in case the element was re-framed).
> 
> r=mats with that addressed (or dismissed if it's not an issue)

I did that.
https://hg.mozilla.org/integration/mozilla-inbound/rev/310cc26b4082

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/310cc26b4082

FWIW: It looks like that cset thinks it touched many more files than it actually did (every file in dom/events, looks like). (I think that can be caused by running old-ish hg versions and using 'hg rebase'.)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

(In reply to Daniel Holbert [:dholbert] from comment #11)
> (I think that can be caused by running old-ish hg versions and using 'hg rebase'.)

Correct.

Roc, please update your Hg ASAP.

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/310cc26b4082

Comment 14

[Ed Morley [:emorley]]

CC :gps re comment 11 and comment 13 (old hg versions and the rebase bug). Any way we can enforce this server side?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I've updated to 2.8.1

Comment 16

[Gregory Szorc [:gps]]

The Mercurial wire protocol intentionally doesn't do version sniffing (in the same way User-Agent sniffing on the web is considered bad form). The wire protocol is client driven and the server sees very little.

To enforce anything would require a custom extension on both the client and server. See bug 941856 for an experiment where I wrote said extension.

Until such an extension is deployed, the best we could do would be to have configure and/or mach sniff the version. If you think that's worth it, file a Core :: Build Config bug and we'll come up with something.

Comment 17

[(no longer active)]

(In reply to comment #16)
> The Mercurial wire protocol intentionally doesn't do version sniffing (in the
> same way User-Agent sniffing on the web is considered bad form). The wire
> protocol is client driven and the server sees very little.
> 
> To enforce anything would require a custom extension on both the client and
> server. See bug 941856 for an experiment where I wrote said extension.

Sigh, an extension won't be good enough.

> Until such an extension is deployed, the best we could do would be to have
> configure and/or mach sniff the version. If you think that's worth it, file a
> Core :: Build Config bug and we'll come up with something.

Filed bug 961258.  Thanks!

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8358202 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 935508
User impact if declined: Crashes on Google graph page
Testing completed (on m-c, etc.): Just landed
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/824b7285697d

Comment 20

[u279076]

svbabukumar, please verify this is fixed in the latest Nightly and Aurora builds.

Comment 21

[svbabukumar]

Attachment: Aurora_27.jpg

Validated the bug in the latest Nightly and Aurora and can see the fields are not edited
Steps to reproduce:

Step 1: Launch the Aurora version of Browser (Build 28 & 29)
Step 2: Navigate to the URL "https://www.google.com/search?sourceid=chrome&ie=UTF-8&q=sqrt%28xx%2Byy%29%2B3cos%28sqrt%28xx%2By*y%29%29%2B5#q=sqrt(x*x%2By*y)%2B3*cos(sqrt(x*x%2By*y))%2B5"
Step 3: In the graph displayed try to change the value of the X,Y or Z by clicking on the fields


Actual results:
Fields X,Y,Z are non editable


Expected results:
Fields X,Y and Z should be editable

Note: Only in Beta 27 we can see it behavior as expected, User is allowed to change the values of X, Y and Z and browser doesn't crash

Please find attached the screen shot

Comment 22

[u279076]

I believe this bug was only intended to address the crash. The fact that the fields remain uneditable deserves a separate bug report. Roc, what do you think?

Comment 23

[Daniel Holbert [:dholbert]]

That's indeed a separate bug.

But, I think it's actually a website bug, not a Mozilla bug. Firstly: yes, Chrome lets you edit those fields, Firefox does not. However, "Inspect Element" in Chrome shows that those elements are <input type="number"> (so, they allow user input), but in Firefox, Inspect Element shows that they're just <td> elements with text.  So it looks like Google is serving different content (editable <input>s to chrome, static content to Firefox).

But yes, in any case, definitely not something to fix as part of this bug.

Comment 24

[u279076]

Thanks Daniel. Marking this bug verified fixed based on comment 21. 

@svbabukumar, as per comment 23 we do not believe this to be something we can fix on our end. That said, if you'd like to file a Tech Evangelism bug, please go ahead and do so: https://bugzilla.mozilla.org/enter_bug.cgi?product=Tech%20Evangelism

Comment 25

[Daniel Holbert [:dholbert]]

(Also, to be fair, it looks like we don't yet support <input type="number"> in release builds, from my testing -- so Google might be waiting to add <input type="number> to this content for Firefox-flavored browsers until the Firefox release version supports that.)