Last Comment Bug 758589 - "ASSERTION: Must have view manager" with mozRequestFullScreen, contenteditable, selection.toString
: "ASSERTION: Must have view manager" with mozRequestFullScreen, contenteditabl...
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: Selection (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mats Palmgren (:mats)
:
Mentors:
Depends on:
Blocks: 326633 748961
  Show dependency treegraph
 
Reported: 2012-05-25 05:50 PDT by Jesse Ruderman
Modified: 2012-05-31 06:19 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (requires focus) (click the button) (357 bytes, text/html)
2012-05-25 05:50 PDT, Jesse Ruderman
no flags Details
stack trace (10.23 KB, text/plain)
2012-05-25 05:51 PDT, Jesse Ruderman
no flags Details
fix (1.30 KB, patch)
2012-05-25 12:22 PDT, Mats Palmgren (:mats)
bugs: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2012-05-25 05:50:45 PDT
Created attachment 627202 [details]
testcase (requires focus) (click the button)

###!!! ASSERTION: Must have view manager: '!isSafeToFlush || mViewManager', file layout/base/nsPresShell.cpp, line 3719

Might be a regression from bug 748961, which added a Flush_Style to nsTypedSelection::ToString.

These variations do *not* assert:
  Using a fresh selection object        "" + window.getSelection();
  Forcing a full layout flush           document.documentElement.offsetHeight
Comment 1 Jesse Ruderman 2012-05-25 05:51:03 PDT
Created attachment 627203 [details]
stack trace
Comment 2 Mats Palmgren (:mats) 2012-05-25 12:22:18 PDT
Created attachment 627317 [details] [diff] [review]
fix

GetPresShell use nsTypedSelection::mPresShellWeak which isn't nulled
out by DisconnectFromPresShell.  Maybe we should, but let's take this
safe patch for now.

https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=25c7876a569a
Comment 3 Mats Palmgren (:mats) 2012-05-25 12:54:11 PDT
BTW, is Destroy'ing the pres shell expected when entering full screen mode?
Comment 4 Timothy Nikkel (:tnikkel) 2012-05-25 13:22:51 PDT
(In reply to Mats Palmgren [:mats] from comment #3)
> BTW, is Destroy'ing the pres shell expected when entering full screen mode?

I think so, we change the position or display of some ancestor of the content document in chrome when we go into fullscreen, so it gets reframed.
Comment 5 Olli Pettay [:smaug] (vacation Aug 25-28) 2012-05-27 10:06:35 PDT
Comment on attachment 627317 [details] [diff] [review]
fix

This is safe.
Comment 7 Phil Ringnalda (:philor) 2012-05-28 22:56:09 PDT
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/cf9f2ec36992 despite probably being harmless and just keeping bad company in that push. Something in that push caused bug 759243, which horrifies me so much I wanted to kill it with as much fire as possible.
Comment 9 Ed Morley [:emorley] 2012-05-31 06:19:18 PDT
https://hg.mozilla.org/mozilla-central/rev/3a67959f7dce

Note You need to log in before you can comment on or make changes to this bug.