530939 - New crash [@ GenerateFlatTextContent] in Firefox 3.6b3 and [@ nsQueryContentEventHandler::GenerateFlatTextContent(nsIRange*, nsString&) ] on 1.9.0

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Johnny Stenback (:jst)]

There's a new crash in Firefox 3.6b3 with the signature "GenerateFlatTextContent" in Firefox 3.6b3 that hasn't been seen in any of the versions 3\.5.*.

Comment 1

[Makoto Kato [:m_kato]]

3.5.x issue is
http://crash-stats.mozilla.com/query/query?do_query=1&product=Firefox&version=Firefox%3A3.5.5&query_search=signature&query_type=startswith&query=nsQueryContentEventHandler%3A%3AGenerateFlatTextContent

I think that startNode is null, so we should check whether startNode or endNode is null or not.

Comment 2

[Makoto Kato [:m_kato]]

Attachment: patch v1

Comment 3

[timeless]

UUID	9e9cf339-8a74-490b-9558-c98972091122
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0
User Comments	갑작스러운 프로그램 종료
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsQueryContentEventHandler::GenerateFlatTextContent 	content/events/src/nsQueryContentEventHandler.cpp:201
1 	xul.dll 	nsQueryContentEventHandler::GetFlatTextOffsetOfRange 	content/events/src/nsQueryContentEventHandler.cpp:549
2 	xul.dll 	nsQueryContentEventHandler::OnQuerySelectedText 	content/events/src/nsQueryContentEventHandler.cpp:381

UUID	a9164e02-a07e-4db3-ab77-6ac3e2091124
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0
User Comments	fuzakennna
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsQueryContentEventHandler::GenerateFlatTextContent 	content/events/src/nsQueryContentEventHandler.cpp:201
1 	xul.dll 	CompositeDataSourceImpl::Unassert 	rdf/base/src/nsCompositeDataSource.cpp:963

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like mFirstSelectedRange's can be null. If so, the endNode might be null because startNode null checking was done in nsContentEventHandler::Init. So, maybe, we should check the endNode in Init() too.

However, I wonder why they can be null...

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 414456 [details] [diff] [review]
patch v1

a=jst

Comment 6

[Makoto Kato [:m_kato]]

landed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/15c46082297d

Comment 7

[Makoto Kato [:m_kato]]

landed to mozilla-1.9.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1ff1e4db21f2

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Kato-san, shouldn't we land this to 1.9.1 branch too?

# Note that nsContentEventHandler was renamed after 1.9.1, the old name is nsQueryContentEventHandler.

Comment 9

[Makoto Kato [:m_kato]]

We should need this since this occurs on 3.5.5 (http://crash-stats.mozilla.com/query/query?product=Firefox&version=Firefox%3A3.5.5&date=&range_value=1&range_unit=weeks&query_search=signature&query_type=exact&query=&do_query=1).

Also, after fixing this on 3.6 and m-c, there is no report for this crash.

Comment 10

[Makoto Kato [:m_kato]]

Attachment: patch for 1.9.1 tree

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 416602 [details] [diff] [review]
patch for 1.9.1 tree

This is for 1.9.1 tree.  Many same crashes are reported on 3.5.5

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 416602 [details] [diff] [review]
patch for 1.9.1 tree

Many CJK users are reporting this crash when using IME.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 416602 [details] [diff] [review]
patch for 1.9.1 tree

Approved for 1.9.1.8, a=dveditz for release-drivers

Comment 14

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ab7aa62cd3df

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Crash reporter doesn't list any crashes for 3.5.8pre builds anymore. But to make sure that it has been fixed I would like to see a testcase. Makato, could you give us some steps which can be used to put Firefox in that crashing situation?

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=6610

This bug report is similar. Kato-san, can you use the testcase on branch? (The testcases don't work fine on trunk due to bug 125282.)

Comment 17

[Makoto Kato [:m_kato]]

(In reply to comment #15)
> Crash reporter doesn't list any crashes for 3.5.8pre builds anymore. But to
> make sure that it has been fixed I would like to see a testcase. Makato, could
> you give us some steps which can be used to put Firefox in that crashing
> situation?

I don't know repro step.  This bug is from crash-stat data.

(In reply to comment #16)

> This bug report is similar. Kato-san, can you use the testcase on branch?
> (The testcases don't work fine on trunk due to bug 125282.)

Thank you, Nakano-san.  But, although I try using test case on Japanese community server, I cannot reproduce this issue on Firefox 3.5.7 + Windows (IME2003) and Mac OS X (Kotoeri).

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Masayuki, Makoto, is this bug the same crash as these: https://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&date=2010-09-07%2020%3A00%3A00&signature=nsQueryContentEventHandler%3A%3AGenerateFlatTextContent(nsIRange*%2C%20nsString%26)&version=Camino%3A2.0.4 ? It looks like it to me.

If so, I'd like to see about taking this on 1.9.0, since Camino still is releasing from there.

Comment 19

[Makoto Kato [:m_kato]]

(In reply to comment #18)
> Masayuki, Makoto, is this bug the same crash as these:
> https://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&date=2010-09-07%2020%3A00%3A00&signature=nsQueryContentEventHandler%3A%3AGenerateFlatTextContent(nsIRange*%2C%20nsString%26)&version=Camino%3A2.0.4
> ? It looks like it to me.
> 
> If so, I'd like to see about taking this on 1.9.0, since Camino still is
> releasing from there.

startNode and endNode seem to be NULL.  I believe that this is same issue.  So, this will be fixed by porting to 1.9.0 tree.

Comment 20

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 416602 [details] [diff] [review]
patch for 1.9.1 tree

(In reply to comment #19)
> startNode and endNode seem to be NULL.  I believe that this is same issue.  So,
> this will be fixed by porting to 1.9.0 tree.

Thanks!  The 1.9.1 patch applies and builds on 1.9.0, so I'll just request approval1.9.0.next on it.

(I haven't found any STR or testcase in the comments from our crashes, either, so we'll just have to trust it works as well on 1.9.0 as on 1.9.1/1.9.2 :) )