1146883 - nightly crashes when deleting quoted html elements in a gmail reply

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

I was trying to reply to the original message in this thread today via gmail:

  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/35_QSL1ABTY

(I was looking at an email, though, not the google group.)

I was trying to trim away excess quote content when the browser crashed.  It seems trying to delete the html elements in the quote triggered the problem.

Here's the ID from about:crashes, but it doesn't seem to be available to view in crash-stats yet:

  8ef32064-eb9a-43a9-a1af-7c54d50f5753

Without the stack I'm not sure what component to put this in yet.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

I guess the crash ID is invalid.

If anyone wants to investigate, let me know and I'll forward the email in question to you.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Can you reproduce reliably?  If so, want to just reproduce in a debug build?

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Ok.  Not sure when I will have to do this.  Might not be today.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Or, I could finally see the dialog box that says "submit crash report" and click ok.

https://crash-stats.mozilla.com/report/index/4faa5c27-5a36-404c-b9ca-7cdb12150324

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

More context:

I first tried selecting a large block of the quote to delete in bulk, but the selection kept getting lost.  So I fell back to just holding the backspace key down for a really long time.  I guess I deleted an important character because that's when the nightly windows went away.

Comment 6

[(no longer active)]

It seems like bug 1088054 accidentally removed a null check here: <http://hg.mozilla.org/mozilla-central/rev/4c792bf9bc28#l2.53>

Comment 7

[(no longer active)]

[Tracking Requested - why for this release]:
This is a regression from bug 1088054 which was landed in the 36 cycle.

Comment 8

[(no longer active)]

Attachment: Null check the node passed to GetGoodSelPointForNode

Comment 9

[(no longer active)]

This seems to be the #15 top crashes in 36.0.4. :(

https://crash-stats.mozilla.com/topcrasher/products/Firefox/versions/36.0.4?days=7

KaiRo, how can I know how serious this crash is?

Comment 10

[(no longer active)]

Attachment: Null check the node passed to GetGoodSelPointForNode

Err, this patch actually compiles!

Comment 11

[Robert Kaiser]

All I know is what you posted already, that it's #15 on 36.0.4 with ~0.6% of all crashes, which isn't too much in volume. That said, it seems to not exist in 35 or older versions. Also, on pre-release channels it's not in the top 30.

Comment 12

[(no longer active)]

(In reply to Robert Kaiser (:[e-mail]) from comment #11)
> All I know is what you posted already, that it's #15 on 36.0.4 with ~0.6% of
> all crashes, which isn't too much in volume. That said, it seems to not
> exist in 35 or older versions. Also, on pre-release channels it's not in the
> top 30.

Yeah that completely matches with bug 1088054 having regressed it.  Thanks!

Comment 13

[(no longer active)]

Comment on attachment 8582502 [details] [diff] [review]
Null check the node passed to GetGoodSelPointForNode

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1088054
User impact if declined: This causes crashes in websites as as webmail or blog editing software.
Testing completed: Locally
Risk to taking this patch (and alternatives if risky): This is essentially risk free, as the fix is a simple null check that takes us to the behavior we had prior to bug 1088054.
String or UUID changes made by this patch: None.

Approval Request Comment
[Feature/regressing bug #]: Not going to repeat the same thing twice.  :-)
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 14

[Lawrence Mandel [:lmandel] (use needinfo)]

Tracking this crash that is close to the top of the top crash list for 36.

Comment 15

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5cee922ae79

Comment 16

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8582502 [details] [diff] [review]
Null check the node passed to GetGoodSelPointForNode

We're going to take this very simple crash fix in 37 RC. I am clearing the b2g37 nom as this will be automerged to that branch. I am also marking release- as we're not going to do another point release for 36. Beta+ Aurora+

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8fda35675a3f

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9a8251ea2aa3

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/8fda35675a3f

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5cee922ae79

Comment 22

[Petruta Horea [:phorea], Desktop Test Engineering]

Reproduced the initial crash and verified it as fixed using Firefox 37 RC build, Firefox 38 beta 1 and Developer Edition 39.0a2 2015-03-30 (64-bit) under Win 7 64-bit and Mac OS X 10.9.5.

The crash-stats shows only 2 crashes after the fix landed, both for 38.0a2 version, so I'm marking this version as verified yet. It will be checked later in the cycle.

Comment 23

[Petruta Horea [:phorea], Desktop Test Engineering]

No new crashes lately for Firefox 38, but there are some new reports for 37.0.2, 39.0a2 and 40.0a1.

Robert, should we file a new bug for that ones?
https://crash-stats.mozilla.com/report/index/47db6d1c-2460-4c2b-b1d9-0b4932150424
https://crash-stats.mozilla.com/report/index/bc323988-f597-40f4-b4a0-20fdd2150423
https://crash-stats.mozilla.com/report/index/e38cbe46-7ea3-4d39-bbc6-92f242150423

Comment 24

[Robert Kaiser]

(In reply to Petruta Rasa [QA] [:petruta] from comment #23)
> Robert, should we file a new bug for that ones?

Looks like the stacks are different, so probably a new bug. That said, looks like the volume of those is not that huge for now.

Comment 25

[(no longer active)]

Can you please file a bug for this and CC me on it?

Comment 26

[Petruta Horea [:phorea], Desktop Test Engineering]

Logged bug 1158837.