Closed Bug 1206483 Opened 8 years ago Closed 8 years ago

[e10s] Crash in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool)

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Version:
43 Branch
Platform:
x86_64
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox43 --- affected

People

(Reporter: over68, Assigned: jorgk-bmo)

References

Details

(Keywords: crash)

Crash Data

Attachments

(3 files, 1 obsolete file)

Call stack at crash time. Sorry about the ZIP, I didn't want to attach 600 KB
6.53 KB, application/zip
Details
Simple test page showing the problem
125 bytes, text/html
Details
Shifting the "offending" code into the caller fixes the crash (v2).
3.25 KB, patch
Details | Diff | Splinter Review 
Steps to reproduce:

1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/ebpwfnkvl0.html.
2. Click on the <select> element.


Crash report: bp-2920619e-69b0-4e5d-91e3-d8f562150919
Flags: needinfo?(alice0775)
Suspect: Bug 772796
Blocks: 772796
Severity: normal → critical
Crash Signature: [@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool)]
Flags: needinfo?(alice0775) → needinfo?(mozilla)
Keywords: crash
[Tracking Requested - why for this release]:

Nightly crashes when double click <select>
Status: UNCONFIRMED → NEW
status-firefox43: --- → affected
tracking-firefox43: --- → ?
Ever confirmed: true
"My" bug 772796 had something to do with the M-C editor. Click somewhere and then press delete. Nothing was changed in the click behaviour. What does the click to behind the scenes?
<!DOCTYPE html>
<html>
<head><!-- CDN hosted by Cachefly -->
<script src="//tinymce.cachefly.net/4.2/tinymce.min.js"></script>
<script>tinymce.init({selector:'textarea'});</script>
</head>
<body>
    <textarea><select><option>Click Here</option></select></textarea>
</body>
</html>

You don't want me to run through your JS. Can you present a self-contained example?

Looking at:
https://crash-stats.mozilla.com/report/index/2920619e-69b0-4e5d-91e3-d8f562150919
it mentions the code I changed: nsHTMLEditRules::GetNodesForOperation, nsEditor::SplitNode.

When I go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/ebpwfnkvl0.html and click on "Chick Here" (which is the <select> element you're talking about), I don't get a crash using a local version compiled from a few days ago. I'll try a Nightly.
Flags: needinfo?(mozilla) → needinfo?(over68)
Some notes here:

1. This crash occurs only with e10s enabled.

2. I discovered this bug when add the <select> element in one of the HTML editors then clicking on it.

See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/5ri7erjp1p.png
Flags: needinfo?(over68)
Nightly crashes when *double* click <select>
OR
Nightly crashes when click <select> and then Click empty erea

TinyMCE and CKEditor are affected.

Alternative STR:
1. Open CKEditor ritch text editor( http://ckeditor.com/demo#full )
2. Click [Source] button
3. Select All(Ctrl+A) and delete [Del]
4. Type <h1><select><option>Click Here</option></select></h1>
5. Click the [Source] button again
6. Double click the <select> element



Alternative STR:
1. Open TinyMCE ritch text editor( http://www.tinymce.com/tryit/basic.php )
2. [Tools] > [<>Source code]
3. Select All(Ctrl+A) and delete [Del]
4. Type <select><option>Click Here</option></select>
5. [OK]
6. Double click the <select> element
   OR
   Click the <select> element and then click empty erea of the ritch text editor

Via local build(win32 build),
Last Good: 01d4b53ea438
First Bad: af909b481b95

Regressed by: af909b481b95	Jorg K — Bug 772796 - Handle newlines correctly when joining <div> and <pre>. r=roc
Nightly on Ubuntu14.04 also crashes (it takes about 5-6 sec after double ckick)
bp-5efad3ca-d9a9-4cd4-88b8-2968d2150920
OS: Windows 7 → All
Forget all the complicated STR. Open this and double-click.
Flags: needinfo?(over68)
Flags: needinfo?(alice0775)
tracking-e10s: --- → ?
tracking-firefox43: ? → ---
Some more debugging using e10s. This is the stack when we first get to:
nsHTMLEditRules::GetNodesForOperation(<args>) Line 5818	C++

xul.dll!nsHTMLEditRules::GetNodesForOperation(<args>) Line 5818	C++
xul.dll!nsHTMLEditRules::GetNodesFromSelection(<args>) Line 6308	C++
xul.dll!nsHTMLEditRules::GetListActionNodes(<args>) Line 5988	C++
xul.dll!nsHTMLEditRules::GetListState(<args>) Line 702	C++
xul.dll!nsHTMLEditor::GetListState(<args>) Line 1864	C++
xul.dll!GetListState(<args>) Line 1481	C++
xul.dll!nsRemoveListCommand::IsCommandEnabled(<args>) Line 407	C++
xul.dll!nsControllerCommandTable::IsCommandEnabled(<args>) Line 99	C++
xul.dll!nsBaseCommandController::IsCommandEnabled(<args>) Line 105	C++
xul.dll!nsWindowRoot::GetEnabledDisabledCommandsForControllers(<args>) Line 323	C++
xul.dll!nsWindowRoot::GetEnabledDisabledCommands(<args>) Line 351	C++
xul.dll!ChildCommandDispatcher::Run(<args>) Line 9701	C++
xul.dll!nsContentUtils::AddScriptRunner(<args>) Line 5235	C++
xul.dll!nsGlobalWindow::UpdateCommands(<args>) Line 9735	C++
xul.dll!nsDocViewerSelectionListener::NotifySelectionChanged(<args>) Line 3580	C++
xul.dll!mozilla::dom::Selection::NotifySelectionListeners(<args>) Line 5987	C++
xul.dll!nsFrameSelection::NotifySelectionListeners(<args>) Line 2266	C++
xul.dll!nsFrameSelection::TakeFocus(<args>) Line 1788	C++
xul.dll!nsFrameSelection::HandleClick(<args>) Line 1557	C++
xul.dll!nsFrame::PeekBackwardAndForward(<args>) Line 3228	C++
xul.dll!nsFrame::SelectByTypeAtPoint(<args>) Line 3111	C++
xul.dll!nsFrame::HandleMultiplePress(<args>) Line 3162	C++
xul.dll!nsFrame::HandlePress(<args>) Line 2947	C++
xul.dll!nsFrame::HandleEvent(<args>) Line 2620	C++
xul.dll!nsComboboxControlFrame::HandleEvent(<args>) Line 1150	C++
xul.dll!nsPresShellEventCB::HandleEvent(<args>) Line 507	C++
xul.dll!mozilla::EventTargetChainItem::HandleEventTargetChain(<args>) Line 361	C++
xul.dll!mozilla::EventDispatcher::Dispatch(<args>) Line 652	C++
xul.dll!PresShell::DispatchEventToDOM(<args>) Line 8067	C++
xul.dll!PresShell::HandleEventInternal(<args>) Line 7961	C++
xul.dll!PresShell::HandlePositionedEvent(<args>) Line 7795	C++
xul.dll!PresShell::HandleEvent(<args>) Line 7581	C++
xul.dll!nsViewManager::DispatchEvent(<args>) Line 797	C++
xul.dll!nsView::HandleEvent(<args>) Line 1128	C++
xul.dll!mozilla::widget::PuppetWidget::DispatchEvent(<args>) Line 324	C++
xul.dll!mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(<args>) Line 489	C++
xul.dll!mozilla::dom::TabChild::RecvRealMouseButtonEvent(<args>) Line 1936	C++
xul.dll!mozilla::dom::PBrowserChild::OnMessageReceived(<args>) Line 3527	C++
xul.dll!mozilla::dom::PContentChild::OnMessageReceived(<args>) Line 5517	C++
xul.dll!mozilla::ipc::MessageChannel::DispatchAsyncMessage(<args>) Line 1388	C++
xul.dll!mozilla::ipc::MessageChannel::DispatchMessageW(<args>) Line 1309	C++
xul.dll!mozilla::ipc::MessageChannel::OnMaybeDequeueOne(<args>) Line 1281	C++
xul.dll!DispatchToMethod<mozilla::ipc::MessageChannel,bool (<args>) Line 388	C++
xul.dll!RunnableMethod<mozilla::ipc::MessageChannel,bool (<args>) Line 323	C++
xul.dll!mozilla::ipc::MessageChannel::RefCountedTask::Run(<args>) Line 469	C++
xul.dll!mozilla::ipc::MessageChannel::DequeueTask::Run(<args>) Line 486	C++
xul.dll!MessageLoop::RunTask(<args>) Line 365	C++
xul.dll!MessageLoop::DeferOrRunPendingTask(<args>) Line 375	C++
xul.dll!MessageLoop::DoWork(<args>) Line 459	C++
xul.dll!mozilla::ipc::DoWorkRunnable::Run(<args>) Line 221	C++
xul.dll!nsThread::ProcessNextEvent(<args>) Line 950	C++
xul.dll!NS_ProcessNextEvent(<args>) Line 277	C++
xul.dll!mozilla::ipc::MessagePump::Run(<args>) Line 127	C++
xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(<args>) Line 290	C++
xul.dll!MessageLoop::RunInternal(<args>) Line 235	C++
xul.dll!MessageLoop::RunHandler(<args>) Line 228	C++
xul.dll!MessageLoop::Run(<args>) Line 202	C++
xul.dll!nsBaseAppShell::Run(<args>) Line 158	C++
xul.dll!nsAppShell::Run(<args>) Line 178	C++
xul.dll!XRE_RunAppShell(<args>) Line 785	C++
xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(<args>) Line 259	C++
xul.dll!MessageLoop::RunInternal(<args>) Line 235	C++
xul.dll!MessageLoop::RunHandler(<args>) Line 228	C++
xul.dll!MessageLoop::Run(<args>) Line 202	C++
xul.dll!XRE_InitChildProcess(<args>) Line 625	C++
plugin-container.exe!content_process_main(<args>) Line 237	C++
plugin-container.exe!NS_internal_main(<args>) Line 11	C++
plugin-container.exe!wmain(<args>) Line 131	C++

Will look into it further.
I pulled out the patch from bug 772796 and the crash goes away. Hmm.

Looks like nsHTMLEditRules::GetNodesForOperation() gets called in a context we didn't expect.
Flags: needinfo?(timdream)
Flags: needinfo?(bugs)
I ran the attached example (attachment 8663398 [details]) in both non-e10s and e10s mode.
In non-e10s mode, nsHTMLEditRules::GetNodesForOperation() does not get called.
In e10s mode, it gets called all the time, the text at the end range is:
|Double-click Here| (length 17), the offset in the range is 17.
I don't understand why this code is called in e10s mode but not in non-e10s mode.

Anyway, I shifted the "offending" code from nsHTMLEditRules::GetNodesForOperation() into its caller nsHTMLEditRules::GetNodesFromPoint().

This fixes the crash. Of course the test for bug 772796 still passes.
Attachment #8663454 - Flags: review?(roc)
Assignee: nobody → mozilla
Oops, didn't do "hg qref" before attaching the patch. Sorry.
Attachment #8663454 - Attachment is obsolete: true
Attachment #8663454 - Flags: review?(roc)
Attachment #8663455 - Flags: review?(roc)
Comment on attachment 8663455 [details] [diff] [review]
Shifting the "offending" code into the caller fixes the crash (v2).

Review of attachment 8663455 [details] [diff] [review]:
-----------------------------------------------------------------

Can you explain what's happening in detail and why this is the right fix?
Attachment #8663455 - Flags: review?(roc)
To fix bug 772796 you suggested to change nsHTMLEditRules::GetPromotedPoint so it returned an offset into the text (bug 772796 comment #111) without changing the DOM.

I then put the slicing up, that is the DOM change, into nsHTMLEditRules::GetNodesForOperation().

Let's recall what GetNodesFromPoint, the work-horse of MoveBlock does:
  PromoteRange() (which calls GetPromotedPoint)
followed by
  GetNodesForOperation (where we put the slicing).

The "offending slicing code" was in GetNodesForOperation (as you had suggested in bug 772796 comment #104). This function has other callers and under certain circumstances text slicing is undesired.

So all I did is the obvious: Moved the offending code into the caller. GetNodesFromPoint now does:
- PromoteRange() (which calls GetPromotedPoint)
- "text slicing"
- GetNodesForOperation (which no longer does the slicing).

That's the right fix, isn't it? Can you please approve.

Can you also please shed some light onto the e10s/non-e10s issue?
Should we backout bug 772796 for now, so that aurora (43) wouldn't have this issue, and then reland it with the fix for this bug? Aurora 43 branch date is today.
Though, it is possibly too late to back this out from m-i, in which case backout would need to happen
on mozilla-aurora.
That's possible. I had hoped Robert would approve quickly, but is hasn't happened.
How do you go about backing this out?
We haven't merged to aurora yet, so I've backed this out of mozilla-central in this commit:

https://hg.mozilla.org/mozilla-central/rev/d49db60f3894
Good, thanks, so this bug can be closed then?
Flags: needinfo?(roc) → needinfo?(nigelbabu)
sure, and bug 772796 should be reopened.
Bug 772796 is reopened, I'll close this one.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Flags: needinfo?(nigelbabu)
tracking-e10s: ? → ---
Component: General → Editor
QA Whiteboard: [good first verify]
Reproduced the bug on firefox nightly 43.0a1 (2015-09-20) on windows 10 x64

Verified as fixed with latest firefox beta 43.0b7 (Build ID: 20151126120800)

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0
QA Whiteboard: [good first verify] → [good first verify] [testday-20151127]
Reproduced this bug by following comment 0 with Firefox Nightly 43.0a1 (2015-09-20); 
(Build ID: 20150920030217) on Linux, 64 Bit

This Bug is now verified as fixed on Latest Firefox Beta 43.0b7

Build ID 	20151126120800
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0
As per comment 5, this is an e10s crash only; Nazir and/or Rezaul, could you please confirm this fix with latest Nightly 45.0a1 too? Thanks in advance!
Flags: needinfo?(na.sb1151)
This Bug's fix is now confirmed as verified on Latest Firefox Nightly 45.0a1 (2015-12-02) 

Build ID: 20151202030210
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Flags: needinfo?(na.sb1151)
You need to log in before you can comment on or make changes to this bug.